Skip to content

Add support for multiple coord_systems #6536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 61 commits into from
Jul 21, 2025

Conversation

ukmo-ccbunney
Copy link
Contributor

@ukmo-ccbunney ukmo-ccbunney commented Jun 26, 2025

🚀 Pull Request

Description

Fixes: #3388

Provides handling of extended CF "grid_mapping" allowing Cube to have multiple coordinate systems associated with it's DimCoords and/or AuxCoords.

From the CF docs:

The grid mapping variables are associated with the data and coordinate variables by the grid_mapping attribute. This attribute is attached to data variables so that variables with different mappings may be present in a single file. The attribute takes a string value with two possible formats. In the first format, it is a single word, which names a grid mapping variable. In the second format, it is a blank-separated list of words ": [ …​] [: …​]" , which identifies one or more grid mapping variables, and with each grid mapping associates one or more coordinatesVariables, i.e. coordinate variables or auxiliary coordinate variables.

Tasks:

  • Parsing of extended grid_mapping format
  • Can load cubes with multiple coordinate systes (OSGB cube described in 5.10 of CF Conventions is a good example)
  • Can save cube with multiple coord_systems to a file and set the grid_mapping correctly.
  • Cross reference against the loaded/generated CRS_WKT attribute in both cases.
    • Can cross reference when saving, but not loading.

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

@ukmo-ccbunney ukmo-ccbunney self-assigned this Jun 26, 2025
* upstream/main:
  Fix concatenat_cube for multidimensional concatenation (SciTools#6534)
  Bump scitools/workflows from 2025.06.2 to 2025.06.3 (SciTools#6532)
Copy link

codecov bot commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 83.21168% with 46 lines in your changes missing coverage. Please review.

Project coverage is 89.89%. Comparing base (41cc939) to head (8478bf5).
Report is 1 commits behind head on FEATURE_wkt.

Files with missing lines Patch % Lines
lib/iris/fileformats/netcdf/saver.py 77.77% 22 Missing and 16 partials ⚠️
lib/iris/fileformats/_nc_load_rules/helpers.py 83.33% 2 Missing and 1 partial ⚠️
lib/iris/fileformats/cf.py 92.50% 0 Missing and 3 partials ⚠️
lib/iris/fileformats/_nc_load_rules/actions.py 92.30% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           FEATURE_wkt    #6536    +/-   ##
=============================================
  Coverage        89.88%   89.89%            
=============================================
  Files               90       90            
  Lines            23923    24058   +135     
  Branches          4461     4505    +44     
=============================================
+ Hits             21504    21627   +123     
- Misses            1668     1671     +3     
- Partials           751      760     +9     

☔ 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.

@ukmo-ccbunney ukmo-ccbunney changed the base branch from main to FEATURE_wkt June 27, 2025 13:18
trexfeathers and others added 3 commits June 27, 2025 16:03
…coord_systems

* origin/mutli_coord_systems:
  Some do-nothing reminders about the next release (SciTools#6521)
  Detailed documentation on Load Problems (SciTools#6529)
  Save crs_wkt on netCDF save, take 2 (SciTools#6519)
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

Thanks @ukmo-ccbunney

Sorry my opinions moved around as I kept reading. I recommend reading all comments before acting on any of them!

@ukmo-ccbunney
Copy link
Contributor Author

Saving now orders the coordinates in the grid_mapping attribute as per the WKT axis order (enabled via Future flag); see 8fd4f0c

Note this commit refactors the saver.py::Saver._create_cf_grid_mapping function to reduce complexity (moves the large if..else block handling metadata creation to it's own function).
The tests are failing at the moment due to the Saver Mock getting a bit upset, but pushing this up now so it can be reviewed while I sort out the tests.

@ukmo-ccbunney
Copy link
Contributor Author

ukmo-ccbunney commented Jul 8, 2025

8776d2c removes expected WKT output from the CFL of most tests.
The generation of WKT output is now only expected and tested in test_Saver.py::Test_write_extended_grid_mapping

@trexfeathers
Copy link
Contributor

8776d2c removes expected WKT output from the CFL of most tests. The generation of WKT output is now only expected and tested in test_Saver.py::Test_write_extended_grid_mapping

If you look at the diff with main you can see that the only CDL changes are the ones you have added, so this has worked as intended - we have reversed the unnecessary changes from #6519.

@ukmo-ccbunney
Copy link
Contributor Author

bec7613 brings in a few changes for the ordered_axes attribute:

  • Renames to extended_grid_mapping, as this is a clearer indication of what it is actually describing
  • Stores this is an attribute on the cube as iris_extended_grid_mapping
  • Keeps the property on the cube; simply gets/sets the attribute
    • This attribute is NOT written to the netCDF file when saving.

A few subtleties:

  • the iris_exteneded_grid_mapping attribute on the cube is ONLY set if the loaded data variable has uses extended grid mapping, otherwise it is NOT set to avoid having an unnecessary extra attribute on the cube (which would also impact lots of tests).
  • In test_netcdf__loadsaveattrs.py, special handling of the iris_exteneded_grid_mapping attribute is needed to make sure it is not picked up on the fixtures for the tests (it does not round trip).

@ukmo-ccbunney
Copy link
Contributor Author

dda23fc extends grid_mapping integration tests to include multiple coord system loading/saving

@ukmo-ccbunney ukmo-ccbunney marked this pull request as ready for review July 17, 2025 15:04
@ukmo-ccbunney ukmo-ccbunney added Type: Enhancement Dragon Sub-Task 🦎 https://github.com/orgs/SciTools/projects/19?pane=info labels Jul 17, 2025
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

Super, thanks for the marathon work @ukmo-ccbunney 🏆

@trexfeathers trexfeathers merged commit 8437bb5 into SciTools:FEATURE_wkt Jul 21, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dragon Sub-Task 🦎 https://github.com/orgs/SciTools/projects/19?pane=info Type: Enhancement
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Support loading/saving netCDF with multiple coordinate systems
2 participants