Skip to content
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

WIP: CF convention parser #568

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP: CF convention parser #568

wants to merge 2 commits into from

Conversation

jbusecke
Copy link
Contributor

@jbusecke jbusecke commented Dec 13, 2022

  • Closes parse CF-style "bounds" coordinates into xgcm axes #127
  • Tests added
  • Passes pre-commit run --all-files
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst
  • Double check dependencies if we decide to require cf-xarray

@jbusecke
Copy link
Contributor Author

jbusecke commented Dec 13, 2022

This actually works quite neatly out of the box thanks to cf-xarray.

Using this I can take a CMIP6 dataset:
image

And get a fully functional xgcm grid with a few lines:

kwargs = cf_parser(ds)
grid = Grid(**kwargs)
grid
<xgcm.Grid>
X Axis (periodic, boundary=None):
  * center   x --> outer
  * outer    x_outer --> center
Y Axis (periodic, boundary=None):
  * center   y --> outer
  * outer    y_outer --> center
Z Axis (periodic, boundary=None):
  * center   lev --> outer
  * outer    lev_outer --> center
T Axis (periodic, boundary=None):
  * center   time --> outer
  * outer    time_outer --> center

The parser function modifies the dataset and then passes it + additional kwargs (at the moment only the coords) to the Grid constructor. We could implement this step automatically in the Grid.__init__ if CF conventions are present.

This design follows the ideas layed out in #566 and might also be a good example for #559 (cc @jatkinson1000).

I think once we have #557 done, we can use this PR to refactor the way metadata conventions are parsed, and then add support for SGRID via #559.

@jbusecke
Copy link
Contributor Author

@dcherian I am curious if you have some ideas on testing here. This works well with a single dataset, but I figured you have a lot more experience with edge cases re CF stuff.

@jbusecke
Copy link
Contributor Author

Another general question here would be if we want to make cf-xarray a full or optional dependency. cc @rabernat @dcherian @TomNicholas.

@dcherian
Copy link
Contributor

Hah I think the edge cases are in the CMIP archive :)

@jbusecke
Copy link
Contributor Author

Hah I think the edge cases are in the CMIP archive :)

LOL

import cf_xarray


def cf_parser(ds):
Copy link
Contributor

Choose a reason for hiding this comment

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

We should type hint this, probably with TypedDict

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.

parse CF-style "bounds" coordinates into xgcm axes
3 participants