Skip to content

Conversation

@pawel-wolff
Copy link
Contributor

Description

This PR:

  • Fixes several bugs in the Xarray engine related to dimension discovery, squeezing vs. ensuring dimensions, and transforming dimensions into variable attributes.
  • Provides more consistent behaviour for Xarray engine options related to dimensions and attributes handling - such as extra_dims, ensure_dims, dims_as_attrs - by clearly distinguishing between dimension names and metadata keys, in line with the logic introduced by the dim_roles concept.
  • Introduces the explicit name "<level_per_type>" for the template dimension implemented in the LevelPerTypeDim class. Under this name, the dimension can be referenced in the options such as ensure_dims and dims_as_attrs.
  • Fixes the "mars" profile by removing several duplicates between attrs and variable_attrs, and moves "param" from attrs to variable_attrs.
  • Makes the ensure_dims and dims_as_attrs options no longer exclusive: one can prevent a size-1 dimension from being squeezed by referencing it in ensure_dims, while at the same time promoting a coordinate of that dimension as a variable attribute.

All the above improvements and (slight) changes to Xarray engine behaviour are:

  • Documented in the to_xarray() method docstring.
  • Documented on the page "User guide - Xarray engine - Dimensions".
  • Demonstrated with new notebooks.
  • Covered by new tests.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

… an attribute and kept as a dimension if included both in `dims_as_attrs` and `ensure_dims`

from now on, extra_dims is to declare extra dims while ensure_dims is to declare which size-one dimensions to keep regardless squeezing or dims_as_attrs
… an attribute and kept as a dimension if included both in `dims_as_attrs` and `ensure_dims`

from now on, extra_dims is to declare extra dims while ensure_dims is to declare which size-one dimensions to keep regardless squeezing or dims_as_attrs
…pe>"

fix in xr-engine builder in the case allow_holes=True
…e (handle the dim=<level_per_type> as well), then rename_attrs is applied, if present
fix in dims_as_attrs with allow_holes=True
xr-engine attributes tests added
…param" moved to variable_attrs;

A related test fixed.
Related notebooks titles improved.
Notebook on xr-engine-dims_as_attrs added.
Notebook on xr-engine-squeeze corrected.
Notebook on xr_engine_extra_dims added.
Notebook on xr_engine_dims_as_attrs corrected.
Notebook on xr_engine_remapping added
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 91.08911% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.81%. Comparing base (a0eca61) to head (dd3a684).

Files with missing lines Patch % Lines
tests/xr_engine/test_xr_dims.py 88.00% 3 Missing and 6 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #867      +/-   ##
===========================================
+ Coverage    85.78%   85.81%   +0.03%     
===========================================
  Files          184      184              
  Lines        14369    14468      +99     
  Branches       713      727      +14     
===========================================
+ Hits         12326    12416      +90     
- Misses        1838     1841       +3     
- Partials       205      211       +6     

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants