Skip to content

Remove unittest reliance on optional dependencies#6976

Open
ESadek-MO wants to merge 25 commits intoSciTools:mainfrom
ESadek-MO:core_deps
Open

Remove unittest reliance on optional dependencies#6976
ESadek-MO wants to merge 25 commits intoSciTools:mainfrom
ESadek-MO:core_deps

Conversation

@ESadek-MO
Copy link
Contributor

🚀 Pull Request

Description


Consult Iris pull request check list


Add any of the below labels to trigger actions on this PR:

  • benchmark_this Request that this pull request be benchmarked to check if it introduces performance shifts

@ESadek-MO ESadek-MO requested a review from pp-mo March 11, 2026 21:36
@ESadek-MO
Copy link
Contributor Author

This can be reviewed, all changes are for this, but I messed up branch management so will either need a rebase or a copy over

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.12%. Comparing base (043b0bc) to head (95d6960).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6976      +/-   ##
==========================================
+ Coverage   90.11%   90.12%   +0.01%     
==========================================
  Files          91       91              
  Lines       24912    24951      +39     
  Branches     4675     4683       +8     
==========================================
+ Hits        22449    22488      +39     
  Misses       1684     1684              
  Partials      779      779              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ESadek-MO ESadek-MO marked this pull request as ready for review March 12, 2026 16:01
Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's been a bit of confusion with files that should have moved from unit to integration,
but the old place didn't get deleted.

Otherwise, looking pretty good !

Comment on lines +16 to +25
try:
import stratify

from iris.experimental.stratify import relevel
except ImportError:
stratify = None


@_shared_utils.skip_stratify
class Test:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need to be able to run integration tests with no stratify available?
If not, we don't need all this.

... and if we do need it, then there's a neat standard PyTest way to skip a whole file of tests when an import is unavailable,
called importorskip

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still like this here, as a just in case. However, I don't think refactoring falls within the scope here so much. If I find there's time to do this after the rest of the work has been done, I will, else I think this becomes a standalone issue, perhaps.

_shared_utils.assert_array_equal(series, cube.data)
_shared_utils.assert_array_equal(series.index, expected_index)
expected_index = dim_coord.points[0]
expected_data = cube.data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new version of the test is all OK as the new unit test,
but it seems to have lost a lot, including all the tests 'test_masked', 'test_time_standard', 'test_time_360' and 'test_copy_masked_true'.

Did you mean to add a copy of the old version (or parts of it) in tests/integration ?

Comment on lines 26 to 41
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I think you could replace all this with "importorskip", since IIUC the whole file will be skipped if there is no pandas.

#
# This file is part of Iris and is released under the BSD license.
# See LICENSE in the root of the repository for full licensing details.
"""Tests for :func:`iris.experimental.regrid.regrid_conservative_via_esmpy`."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it doesn't say "unit tests", this doesn't need to be changed to "integration" (!) 👍

BUT.. this should appear as a file move, not a new file.
You need to delete the original lib/iris/tests/experimental/regrid/test_regrid_conservative_via_esmpy.py

@@ -0,0 +1,18 @@
<?xml version="1.0" ?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should appear as a move, not a new file.
Need to delete lib/iris/tests/results/unit/experimental/stratify/relevel/multi_dim_target_levels.cml

#
# This file is part of Iris and is released under the BSD license.
# See LICENSE in the root of the repository for full licensing details.
"""Unit tests for :func:`iris.util.mask_cube_from_shapefile`."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs rename

Suggested change
"""Unit tests for :func:`iris.util.mask_cube_from_shapefile`."""
"""Integration tests for :func:`iris.util.mask_cube_from_shapefile`."""

Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More.
Not sure why it wouldn't let me add this one to the existing review, I'm sure it let me do that with the previous one ??


import numpy as np
import pytest
from shapely.geometry import box
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing wrong with this,
But weirdly, we still have the same line in the original source
tests/unit/util/test_mask_cube_from_shapefile.py
.

So, that will fail when remove the deps, and seems to need fixing somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check I've done this as you meant!

@pp-mo pp-mo linked an issue Mar 16, 2026 that may be closed by this pull request
5 tasks
Copy link
Contributor Author

@ESadek-MO ESadek-MO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also needs a whatsnew!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Only use core dependencies for unit tests

2 participants