Add unit tests for Cube.coord_dims by 01luyicheng · Pull Request #7198 · SciTools/iris

01luyicheng · 2026-07-10T17:48:45Z

Description

Adds specific unit tests for Cube.coord_dims, addressing #4552.

Cube.coord_dims previously had no direct unit tests in lib/iris/tests/unit/cube/test_Cube.py (the existing Coord.cube_dims / CellMeasure.cube_dims / AncillaryVariable.cube_dims tests only mock-verify that those methods delegate to Cube.coord_dims, without exercising coord_dims itself).

This adds a Test_coord_dims class covering the seven scenarios listed in the issue, each exercising a distinct branch of the method:

Scenario (from #4552)	Test
coord on the cube	`test_dim_coord`
coord not on the cube (but matches one that is)	`test_equivalent_coord_not_instance`
coord not on the cube (and doesn't match one that is)	`test_no_match_raises`
string	`test_string_name`
coord on multiple dims	`test_aux_coord_multiple_dims`
coord is aux factory	`test_aux_factory`
coord is aux_coord	`test_aux_coord_single_dim`

Closes #4552.

Checklist

Included a What's New entry — added in changelog/ (follow-up commit)
Incorporated type hints in any changed code — N/A (test-only; existing tests in this file do not use type hints)
Checked if tests need updating — this PR adds the tests
Checked if benchmarks need updating — N/A
Checked if documentation needs updating — N/A
Checked if dependencies need updating — N/A
Confirmed that the GitHub 'checks' on this PR are passing — pending CI

Testing

All 7 new tests pass, and the full test_Cube.py suite shows no regressions:

lib/iris/tests/unit/cube/test_Cube.py::Test_coord_dims ... 7 passed
lib/iris/tests/unit/cube/test_Cube.py ... 497 passed, 34 skipped

ruff check and ruff format --check both pass with no warnings.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

CLAassistant · 2026-07-10T17:48:53Z

All committers have signed the CLA.

stephenworsley · 2026-07-15T09:28:06Z

From @SciTools/peloton: Hi @01luyicheng, thanks for the PR, we'll get this reviewed soon, in the mean time, we just need you to sign the CLA (linked here #7198 (comment)).

01luyicheng · 2026-07-16T11:04:31Z

Hi, the CLA is signed now - looking forward to the review. Thanks!

codecov · 2026-07-23T16:27:36Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.18%. Comparing base (c2ffc9b) to head (29e393d).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7198   +/-   ##
=======================================
  Coverage   90.18%   90.18%           
=======================================
  Files          91       91           
  Lines       25102    25102           
  Branches     4706     4706           
=======================================
  Hits        22638    22638           
  Misses       1685     1685           
  Partials      779      779

☔ View full report in Codecov by Harness.
📢 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

Hi @01luyicheng, thanks so much for tackling this!

I've got a couple of comments; the only problems I've found thus far are readability concerns, the code itself all looks great!

ESadek-MO · 2026-07-23T15:53:24Z

+        self.x_coord = x_coord
+        self.aux_1d = aux_1d
+        self.aux_2d = aux_2d
+        self.factory = factory


See above.

Suggested change

self.factory = factory

ESadek-MO · 2026-07-23T15:57:57Z

+        # Dependencies for a hybrid-height derived coordinate ("altitude").
+        delta = AuxCoord(points=np.array([0, 1]), long_name="delta", units="m")
+        sigma = AuxCoord(points=np.array([0, 1]), long_name="sigma")
+        orography = AuxCoord(
+            np.arange(12).reshape(3, 4), units="m", long_name="orography"
+        )
+        cube.add_aux_coord(delta, 0)
+        cube.add_aux_coord(sigma, 0)
+        cube.add_aux_coord(orography, (1, 2))
+        factory = HybridHeightFactory(delta=delta, sigma=sigma, orography=orography)
+        cube.add_aux_factory(factory)


Since this is only used in one test, it possibly makes sense to move this to the test itself.

Normally, I'd avoid making tests unreadable, but I think in this case it's a clear enough set-up that it should be okay. Alternatively, I could see an argument for making this its own fixture.

ESadek-MO · 2026-07-23T16:11:16Z

+    """Tests for :meth:`iris.cube.Cube.coord_dims`."""
+
+    @pytest.fixture(autouse=True)
+    def _setup(self):


This setup is quite hard to read. I'd suggest breaking it up into code blocks of related bits, separated by lines.

For example, one structure could be:

Set-up the cube

Set-up the DimCoords (and add them to the cube)

Set-up the AuxCoords (and add them to the cube)

Set-up the hybrid heights AuxCoords, and add them to the cube, then set-up the factory. (If this stays in the setup at all)

Assigned them all to class variables.

@ESadek-MO

Address review feedback from @ESadek-MO: - Move hybrid-height set-up into the single test that uses it - Group _setup into clear blocks: cube → DimCoords → AuxCoords → assigns

01luyicheng · 2026-07-23T18:17:58Z

Hi @ESadek-MO,

Thanks so much for the review and the kind words! I've just pushed an update (commit 8d101a2) that addresses both points:

Hybrid-height set-up moved: The delta/sigma/orography/factory construction was only used by test_aux_factory, so I've moved it into that test method rather than keeping it in the shared _setup (and dropped the now-unused self.factory assignment).
_setup restructured: Reorganized into clearly separated, blank-line-delimited blocks — cube construction → DimCoords (and adds) → AuxCoords (and adds) → assignment to class variables — matching the order you suggested.

No test logic or assertions were changed. Let me know if you'd prefer the hybrid-height set-up extracted into its own fixture instead of inlined into the test; happy to switch it over.

Thanks again for your time!

01luyicheng · 2026-07-25T18:29:10Z

Hi @ESadek-MO, just a gentle follow-up on the changes I pushed in 8d101a2 — the _setup is now broken into blank-line-delimited blocks (cube → DimCoords → AuxCoords → class-variable assignment) and the hybrid-height delta/sigma/orography/factory construction has been moved into test_aux_factory, since it's the only test using it. No assertions or test logic were changed, and Codecov is green.

I'm still happy to extract the hybrid-height set-up into a fixture if you'd prefer that over inlining it in the test — just say the word and I'll switch it over. No rush at all on my end; thanks again for the review!

Add unit tests for Cube.coord_dims

671ecc8

Copilot AI review requested due to automatic review settings July 10, 2026 17:48

Copilot AI reviewed Jul 10, 2026

Add What's New changelog fragment for SciTools#4552

c456fdf

scitools-ci Bot added this to 🚴 Peloton Jul 11, 2026

Merge branch 'main' into add-cube-coord-dims-unit-tests

29e393d

ESadek-MO self-assigned this Jul 23, 2026

ESadek-MO requested changes Jul 23, 2026

View reviewed changes

Refactor Test_coord_dims._setup for readability

8d101a2

Address review feedback from @ESadek-MO: - Move hybrid-height set-up into the single test that uses it - Group _setup into clear blocks: cube → DimCoords → AuxCoords → assigns

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add unit tests for Cube.coord_dims#7198

Add unit tests for Cube.coord_dims#7198
01luyicheng wants to merge 4 commits into
SciTools:mainfrom
01luyicheng:add-cube-coord-dims-unit-tests

01luyicheng commented Jul 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

CLAassistant commented Jul 10, 2026 •

edited

Loading

Uh oh!

stephenworsley commented Jul 15, 2026

Uh oh!

01luyicheng commented Jul 16, 2026

Uh oh!

codecov Bot commented Jul 23, 2026

Uh oh!

ESadek-MO left a comment

Uh oh!

ESadek-MO Jul 23, 2026

Uh oh!

ESadek-MO Jul 23, 2026

Uh oh!

ESadek-MO Jul 23, 2026

Uh oh!

01luyicheng commented Jul 23, 2026

Uh oh!

01luyicheng commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

01luyicheng commented Jul 10, 2026

Description

Checklist

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephenworsley commented Jul 15, 2026

Uh oh!

01luyicheng commented Jul 16, 2026

Uh oh!

codecov Bot commented Jul 23, 2026

Codecov Report

Uh oh!

ESadek-MO left a comment

Choose a reason for hiding this comment

Uh oh!

ESadek-MO Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

ESadek-MO Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

ESadek-MO Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

01luyicheng commented Jul 23, 2026

Uh oh!

01luyicheng commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CLAassistant commented Jul 10, 2026 •

edited

Loading