search

xarray-contrib / cf-xarray

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

ENH: Update units in .cf.differentiate #515

Closed DWesl closed 2 weeks ago

DWesl commented 4 weeks ago

Differentiating a field results in a field whose units are the quotient of those of the original field with the units of the field with respect to which the derivative was taken. This PR adds code to update the units of the derivative to reflect that.

The helper function should simplify adding similar functionality for integrate and cumulative_integrate.

Should I add validation that neither unit is the empty string, since that breaks parsing?

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.

Project coverage is 85.51%. Comparing base (a9cebee) to head (4c3ce70). Report is 12 commits behind head on main.

:exclamation: Current head 4c3ce70 differs from pull request most recent head 354b0ea

Please upload reports for the commit 354b0ea to get more accurate results.

Files Patch % Lines
cf_xarray/accessor.py 78.94% 3 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #515 +/- ## ========================================== - Coverage 85.78% 85.51% -0.28% ========================================== Files 13 13 Lines 2364 2395 +31 Branches 183 190 +7 ========================================== + Hits 2028 2048 +20 - Misses 303 313 +10 - Partials 33 34 +1 ``` | [Flag](https://app.codecov.io/gh/xarray-contrib/cf-xarray/pull/515/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xarray-contrib) | Coverage Δ | | |---|---|---| | [mypy](https://app.codecov.io/gh/xarray-contrib/cf-xarray/pull/515/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xarray-contrib) | `38.40% <56.25%> (-0.14%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/xarray-contrib/cf-xarray/pull/515/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xarray-contrib) | `93.62% <64.70%> (-0.37%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xarray-contrib#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dcherian commented 2 weeks ago

Thanks for the PR.

However, I don't think I will merge this. I'd prefer that pint handle these unit transformations instead. Is there a reason pint doesn't work for you?

DWesl commented 2 weeks ago

I suppose I could make my own function to wrap the ds.differentiate() call and use pint to update the units attribute afterwards, but I've gotten used to using the xarray method, and the accessors aren't all that different.

If you'd prefer I move this to pint-xarray rather than cf-xarray, I can do that.

dcherian commented 2 weeks ago

I guess the problem here is that units on indexed coordinates are not supported yet, which would require movement on https://github.com/xarray-contrib/pint-xarray/pull/163. In any case, I think this should be solved with the pint/xarray integration not cf-xarray. If you have units not recognized by pint, then I'm happy to merge a change to fix that.