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 conventions helper #31

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Add cf conventions helper #31

wants to merge 5 commits into from

Conversation

banesullivan
Copy link
Member

@banesullivan banesullivan commented Jul 2, 2022

Resolves #15

I feel this should be double-checked by someone more familiar with CF conventions and how that accessor works.

Also, this should be tested against more datasets

cc @rabernat, any feedback here? The premise is that the new function added here can attempt to use cf_xarray to get the XYZ coordinate array names when the user does not specify names.

In future work, this can be used to address #19

Comment on lines +9 to +11
x = axes.get("X", [None])[0]
y = axes.get("Y", [None])[0]
z = axes.get("Z", [None])[0]
Copy link
Member Author

Choose a reason for hiding this comment

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

This just feels wrong

Choose a reason for hiding this comment

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

I agree that is quite ugly.

Alternatives are

x = da.axes["X"][0] if "X" in da.cf else None

or in _get_array:

# if cf_xarray exists
return da.cf[key]  # this falls back to usually xarray stuff if "key" is not interpretable using cf conventions

Copy link
Member Author

Choose a reason for hiding this comment

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

Great! thank you for the suggestions

@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2022

Codecov Report

Merging #31 (949d7cc) into main (3231ba8) will increase coverage by 0.63%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
+ Coverage   90.73%   91.36%   +0.63%     
==========================================
  Files           8        9       +1     
  Lines         205      220      +15     
==========================================
+ Hits          186      201      +15     
  Misses         19       19              
Impacted Files Coverage Δ
pvxarray/accessor.py 96.29% <100.00%> (+0.46%) ⬆️
pvxarray/cf.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3231ba8...949d7cc. Read the comment docs.

@TomNicholas
Copy link
Collaborator

cc'ing @dcherian as a cf-xarray pro

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.

Utilize cf-xarray
4 participants