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

Take units from cf_xarray #1590

Merged
merged 7 commits into from
Jan 11, 2024
Merged

Take units from cf_xarray #1590

merged 7 commits into from
Jan 11, 2024

Conversation

aulemahal
Copy link
Collaborator

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • The units registry is copied from cf_xarray and extended with xclim's specific shortcuts (precipitation, discharge and the hydro context).
  • The units formatting functions are simplified by using the work alreadty done in cf_xarray. We still can't get rid of pint2cfunits because of pint issue that makes it impossible to format dimensionless units are "" using its formatter tools.

Does this PR introduce a breaking change?

Yes. As seen in the tests changes, cf_xarray's cf formatter never adds the "^" sign for exponentiation. This does not affect the accepted units, only the output units attribute.

@aulemahal aulemahal requested a review from Zeitsperre January 10, 2024 20:14
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Collaborator

@Zeitsperre Zeitsperre left a comment

Choose a reason for hiding this comment

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

Well done!

CHANGES.rst Outdated Show resolved Hide resolved
docs/notebooks/units.ipynb Outdated Show resolved Hide resolved
xclim/core/units.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the approved Approved for additional tests label Jan 10, 2024
@github-actions github-actions bot added sdba Issues concerning the sdba submodule. docs Improvements to documenation labels Jan 10, 2024
@aulemahal aulemahal merged commit 0bb11c2 into master Jan 11, 2024
16 checks passed
@aulemahal aulemahal deleted the use-cfxr-units branch January 11, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests docs Improvements to documenation sdba Issues concerning the sdba submodule.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants