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

add cf.grid_mapping_names #391

Merged
merged 60 commits into from
Feb 8, 2023
Merged

Conversation

larsbuntemeyer
Copy link
Contributor

@larsbuntemeyer larsbuntemeyer commented Jan 23, 2023

I wanted to pick up the discussion from #370 of making cf-xarray understand the grid_mapping attribute. This alone (without the further interpretation by pyproj) would be really useful to RCM datasets (you never know if the mapping is called rotated_pole, rotated_latitude_longitude or whatever...).

The implementation is straight forward, the logic in the CF conventions mainly follows the bounds logic. A data variable can have a grid_mapping attribute that points to another data variable containing the grid_mapping_name attribute. As far as i know, the name of the variable holding the grid_mapping_name attribute might be arbitrary. Actually, it's a lot of code copied from the bounds logic, which i should optimize, sorry... Just wanted to throw this in quickly, let me know if it's useful...

  • add cf.grid_mappings
  • update repl
  • add a new page to the docs titled "Grid Mappings"
  • add tests
  • add DataArray tests
  • update _COORD_NAMES and _AXIS_NAMES
  • Add more See Also sections
  • Update docs

Here's an example:

from cf_xarray.datasets import rotds
rotds.cf
Coordinates:
- CF Axes: * X: ['rlon']
           * Y: ['rlat']
             Z, T: n/a

- CF Coordinates:   longitude: ['lon']
                    latitude: ['lat']
                    vertical, time: n/a

- Cell Measures:   area, volume: n/a

- Standard Names: * grid_latitude: ['rlat']
                  * grid_longitude: ['rlon']
                    latitude: ['lat']
                    longitude: ['lon']

- Bounds:   n/a

- Grid Mappings:   n/a

Data Variables:
- Cell Measures:   area, volume: n/a

- Standard Names:   air_temperature: ['temp']

- Bounds:   lat: ['lat_bounds']
            latitude: ['lat_bounds']
            lon: ['lon_bounds']
            longitude: ['lon_bounds']

- Grid Mappings:   air_temperature: ['rotated_pole']
                   temp: ['rotated_pole']

and...

rotds.cf.grid_mappings
{'air_temperature': ['rotated_pole'], 'temp': ['rotated_pole']}

access it:

rotds.cf.get_grid_mapping("temp")

grafik
get name of the mapping (might be different to the data variable name):

rotds.cf.get_grid_mapping_name("temp")
'rotated_latitude_longitude'

Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

Thanks for contributing @larsbuntemeyer

This is looking good to me. I have a few requests:

  1. Please add a new page to the docs titled "Grid Mappings"
  2. Please add to api.rst
  3. Please add tests
  4. Should we add a specific DataArray property that's more narrow? It seems like it could work since Xarray propagates scalars with all DataArrays when extracted.
  5. That reminds me, we should update get_associated_variables too and check that grid_mapping information is propagated with ds.cf["temp"]

(3), (4) could happen in a separate PR if you prefer.

The tests failues can be fixed by updating the "expected" repr

cf_xarray/accessor.py Outdated Show resolved Hide resolved
cf_xarray/accessor.py Outdated Show resolved Hide resolved
@dcherian
Copy link
Contributor

@jthielen do you have any thoughts on this API, and/or suggestions for API additions that would be useful.

@larsbuntemeyer
Copy link
Contributor Author

  1. Please add a new page to the docs titled "Grid Mappings"

  2. Please add to api.rst

  3. Please add tests

Great, thanks! I'll start polishing!

  1. That reminds me, we should update get_associated_variables too and check that grid_mapping information is propagated with ds.cf["temp"]

Yes, that would be great. I saw that in datasets.airds you explicitly added cell_area to coords. That should also happen for the grid_mapping i guess.

(3), (4) could happen in a separate PR if you prefer.

I'll add tests here...

@dcherian
Copy link
Contributor

Seems like we should add grid_latitude and grid_longitude to _COORD_NAMES too? This would allow ds.cf.coords["grid_latitude"] for example

@larsbuntemeyer
Copy link
Contributor Author

Yes, that would be nice! grid_latitude and grid_longitude are somehow specific to rotated_latitude_longitude. There is also:

  • projection_x_angular_coordinate and projection_y_angular_coordinate for geostationary projection
  • projection_x_coordinate and projection_y_coordinate for all other mappings

Should i add them in this PR?

@dcherian
Copy link
Contributor

Should i add them in this PR?

Sure. I think @jthielen may have commented earlier about not adding these, but I can't find it now.

@larsbuntemeyer
Copy link
Contributor Author

maybe #23 is related.

@dcherian
Copy link
Contributor

maybe #23 is related.

Thanks that's right. I think adding to _COORD_NAMES should be fine. projection_x,y,angular_coordinate should perhaps be under _AXIS_NAMES.

This is quite minor though,

@larsbuntemeyer
Copy link
Contributor Author

larsbuntemeyer commented Jan 24, 2023

I think adding to _COORD_NAMES should be fine. projection_x,y,angular_coordinate should perhaps be under >_AXIS_NAMES.

I just recognized that this would clutter the repr a little bit, e.g.,

Coordinates:
- CF Axes: * X: ['lon']
           * Y: ['lat']
           * T: ['time']
             Z, projection_x_coordinate, projection_y_coordinate, projection_x_angular_coordinate, projection_y_angular_coordinate: n/a

- CF Coordinates: * longitude: ['lon']
                  * latitude: ['lat']
                  * time: ['time']
                    grid_longitude, grid_latitude, vertical: n/a

should this be done in another PR and maybe with an html repr that deals with it?

@dcherian
Copy link
Contributor

dcherian commented Feb 6, 2023

I changed it so that:

  • rotds.cf.grid_mapping_names returns {'rotated_latitude_longitude': ['rotated_pole']}
  • rotds.cf["rotated_latitude_longitude"] returns rotds.rotated_pole
  • rotds.cf["temp"].cf.grid_mapping_name returns 'rotated_latitude_longitude'

I enabled the .cf["grid_mapping"] which will return a DataArray, so that removes the need for DataArray.cf.grid_mapping. Sorry about this, I need to write it down somewhere, but in general, properties return strings and __getitem__ returns arrays. So here I set it so that "grid_mapping" will get us the DataArray.

In the future, we might think about adding a custom GridMapping class, returned by DataArray.cf.grid_mapping that has methods to convert it to WKT or pyproj.CRS or whatever.

Please try it out with your code and see if you like it.

@dcherian dcherian changed the title add cf.grid_mappings add cf.grid_mapping_names Feb 6, 2023
@larsbuntemeyer
Copy link
Contributor Author

Thanks a lot, that's really awesome and makes sense to me, will be really useful for working with CORDEX data! I did some minor updates for the docs!

@larsbuntemeyer
Copy link
Contributor Author

I guess, the error is caused by the CF tables being updated just yesterday!

@dcherian dcherian enabled auto-merge (squash) February 8, 2023 17:57
@dcherian dcherian enabled auto-merge (squash) February 8, 2023 17:59
@dcherian
Copy link
Contributor

dcherian commented Feb 8, 2023

Thanks @larsbuntemeyer this is great!

will be really useful for working with CORDEX data!

If you ever write a demo notebook showcasing this, it would be good to link to it in our docs. That would be a valuable community contribution!

@dcherian dcherian merged commit 5356069 into xarray-contrib:main Feb 8, 2023
dcherian added a commit to dcherian/cf-xarray that referenced this pull request Feb 8, 2023
* upstream/main:
  add `cf.grid_mapping_names` (xarray-contrib#391)
  Update CF standard name table v80 (xarray-contrib#423)
@larsbuntemeyer larsbuntemeyer deleted the grid_mapping branch February 8, 2023 20:44
@larsbuntemeyer
Copy link
Contributor Author

If you ever write a demo notebook showcasing this, it would be good to link to it in our docs.

Absolutely! Thanks for the support!

dcherian added a commit to dcherian/cf-xarray that referenced this pull request Feb 8, 2023
* main:
  Add rich repr (xarray-contrib#409)
  add `cf.grid_mapping_names` (xarray-contrib#391)
  Update CF standard name table v80 (xarray-contrib#423)
dcherian added a commit to Descanonge/cf-xarray that referenced this pull request Feb 22, 2023
* main: (33 commits)
  Update README.rst
  Update README.rst
  v0.8.0 release (xarray-contrib#424)
  Add sgrid axes parsing (xarray-contrib#421)
  Add rich repr (xarray-contrib#409)
  add `cf.grid_mapping_names` (xarray-contrib#391)
  Update CF standard name table v80 (xarray-contrib#423)
  Support grid_topology, mesh_topology CF roles. (xarray-contrib#420)
  added degrees units (xarray-contrib#390)
  Test and support 3.11 (xarray-contrib#417)
  Update link to COSIMA tutorial (xarray-contrib#419)
  Bump mamba-org/provision-with-micromamba from 14 to 15 (xarray-contrib#418)
  Update whats-new.rst (xarray-contrib#414)
  [skip-ci] Include data folder (xarray-contrib#416)
  Update changelog URL
  updated whats new for release (xarray-contrib#413)
  Using regex package for match (xarray-contrib#408)
  Try pytest-pretty (xarray-contrib#410)
  Update CITATION.cff (xarray-contrib#399)
  Fix upstream-dev CI (xarray-contrib#406)
  ...
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