xarray-contrib / cf-xarray

an accessor for xarray objects that interprets CF attributes
https://cf-xarray.readthedocs.io/
Apache License 2.0
157 stars 39 forks source link

Don't look for `T` in sgrid axes #435

Closed kthyng closed 1 year ago

kthyng commented 1 year ago

cf-xarray is not finding T as a time axis due to a KeyError. This first checks to see if the key is in sgrid_axes before using that.

Closes https://github.com/xarray-contrib/cf-xarray/issues/434

kthyng commented 1 year ago

@dcherian the errors don't look related as far as I can tell.

kthyng commented 1 year ago

I'm not sure how other people are using cf-xarray currently because this is a total blocker for me!

codecov[bot] commented 1 year ago

Codecov Report

Merging #435 (0f9c7c6) into main (782c9e3) will decrease coverage by 0.02%. The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main     #435      +/-   ##
==========================================
- Coverage   37.37%   37.35%   -0.02%     
==========================================
  Files          21       21              
  Lines        3914     3916       +2     
  Branches      193      194       +1     
==========================================
  Hits         1463     1463              
- Misses       2258     2259       +1     
- Partials      193      194       +1     
Flag Coverage Δ
mypy 37.35% <33.33%> (-0.02%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cf_xarray/accessor.py 35.96% <0.00%> (ø)
cf_xarray/tests/test_accessor.py 29.17% <50.00%> (+0.03%) :arrow_up:

... and 1 file with indirect coverage changes

dcherian commented 1 year ago

This looks right, can you add a test.

Presumably you have an sgrid dataset with the grid_topology variable, Anyone without it is unaffected

kthyng commented 1 year ago

Ok in modifying a test to check for this I was forced to look into parse_axes some – I'm not familiar with the grid topology stuff at all. But it looks like X, Y, and Z are the only options anyway for key so I put them directly into _get_axis_coord.

Does that work?

dcherian commented 1 year ago

Yes I think that's right.