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

Rewrite unit formatter for pint 0.24 and earlier #523

Merged
merged 5 commits into from
Jul 5, 2024
Merged

Conversation

aulemahal
Copy link
Contributor

@aulemahal aulemahal commented Jun 28, 2024

Fixes #522 and hgrecco/pint#2024.

  • Rewrite of the custom unit formatter to properly use the tools of pint instead of relying on regex.
  • To preserve backwards compatibility the pint.formatter call changes according to the installed version of pint.
  • Before, f"{u:cf}" and f"{u:~cf}" returned the same thing except for "dimensionless". This shortening is done explicitly in the new formatter with a if-else in case it was already done by pint.
  • Removed a few unneeded lines
    • "percent" was always printed as "%" for all supported pint versions, so I don't know why it as there.
    • The formatter's declaration never raised ImportError, so I think the try-except was a legacy from something else.
  • Added some warnings to the documentation about non-udunits units.
  • Added dimensionless to the tests.

I tested this with pint 0.24.1 and 0.23. I also did a very minimal test with pint 0.22 down to 0.12. This seems to support pint >= 0.18, like the previous version did.

With pint < 0.24 the only difference this makes is that dimensionless units are returned as "" with {u:cf} and {u:~cf}, where as the previous version returned "dimensionless" for {u:cf}.

@aulemahal aulemahal requested a review from dcherian June 28, 2024 19:57
@dcherian
Copy link
Contributor

dcherian commented Jul 1, 2024

Failure is real.

@keewis
Copy link
Contributor

keewis commented Jul 1, 2024

I've been a bit away from pint, so I'll have to first look into how it changed in the most recent version before giving any thorough reviews, but at a quick glance this looks fine.

@dcherian
Copy link
Contributor

dcherian commented Jul 1, 2024

but at a quick glance this looks fine.

Thanks! no need for a thorough review.

cf_xarray/units.py Outdated Show resolved Hide resolved
@dcherian dcherian merged commit a74efdb into main Jul 5, 2024
10 checks passed
@dcherian dcherian deleted the fix-units-pint24 branch July 5, 2024 01:06
finam-admin pushed a commit to finam-ufz/finam that referenced this pull request Jul 20, 2024
dcherian added a commit that referenced this pull request Jul 24, 2024
* main: (35 commits)
  Add release.yml
  Allow encoding/decoding multiple geometries (#526)
  Rewrite unit formatter for pint 0.24 and earlier (#523)
  [pre-commit.ci] pre-commit autoupdate (#527)
  Add grid_mapping for geometries if possible (#521)
  Bump pypa/gh-action-pypi-publish from 1.8.14 to 1.9.0 (#524)
  Bump codecov/codecov-action from 4.4.1 to 4.5.0 (#525)
  Add geometry encoding and decoding functions. (#517)
  Bump links to CF-1.11 (#519)
  Docs cleanup (#518)
  Bump codecov/codecov-action from 4.3.0 to 4.4.1 (#514)
  [pre-commit.ci] pre-commit autoupdate (#510)
  Bump pypa/gh-action-pypi-publish from 1.8.12 to 1.8.14 (#509)
  Add docs about converting between shapely and cf (#512)
  Bump codecov/codecov-action from 4.1.0 to 4.3.0 (#511)
  Fix scheduled nightly upstream test
  Bump pypa/gh-action-pypi-publish from 1.8.11 to 1.8.12 (#503)
  Bump codecov/codecov-action from 4.0.0 to 4.1.0 (#504)
  numpy 2 compat (#505)
  ruff settings: move 'ignore' to 'lint' section (#506)
  ...
aulemahal added a commit to Ouranosinc/xclim that referenced this pull request Jul 31, 2024
<!--Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #1785 
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGES.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

### What kind of change does this PR introduce?

* Changes NaN and NAN to nan, Inf to inf.
* Changes a test so the new data type promotion of numpy 2 fits our
expected values
* Relaxes a test for the same reason
* Change expected unit order in some cases (new cf_xarray + pint)
* Dimensionless units are now printed as "1". 
* Simplify `pint2cfunits`.
* Add `ensure_absolute_tempetature` to its module's `__all__` and move
`ensure_delta` up in the same module so both functions are near another
in the file.

### Does this PR introduce a breaking change?
Yes it does. Fixing numpy 2 issues made me fix pint 0.24.1 issues that
made me fix cf_xarray issues which have solution that is not
backwards-compatible and now pint and cf_xarray have updated pinned that
imply a numpy >=2 pin.

### Other information:
~We will require 3 new pins :~
- Most problems with `create_ensemble` come from a np2 bug in xarray,
which was fixed here pydata/xarray#9182. ~We are thus waiting for a
release~. Xarray 2024.07.0 out on the 30th.
- All units problem are solved with xarray-contrib/cf-xarray#523, which
was released in cf-xarray 0.9.3.
- The dimensionless unit thing requires pint 0.24.1 ~which requires
numpy 2, so pinning this as well.~

UPDATE: No pins were added, but the behaviour of xclim will be different
for dimensionless indicators depending on the cf_xarray/pint versions
installed.
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.

CF unit formatter incompatible with latest pint
4 participants