MetPy icon indicating copy to clipboard operation
MetPy copied to clipboard

Improve setup of pint UnitRegistry

Open dopplershift opened this issue 4 years ago • 5 comments

There are now functions for set_application_registry() and get_application_registry(), so we should really look at using those for a registry. This would allow us to play nicely with other libraries/applications using pint, such as xarray-pint. Questions:

  • We currently add our own units and loosen the temperature unit rules. What do we do about this?
  • Should we modify existing registries?
  • Should we ever set the application registry?

dopplershift avatar Apr 14 '20 05:04 dopplershift

If we were just adding our own units, then I don't think it would hurt to use get_application_registry() and just add our units. However, I think the loosening of the temperature unit rules and adding UDUNITS-style power compatibility are important to have, but also dangerous to try including in a user's existing registry. So, my opinion would be that we still need to define our own registry, and not modify existing registries.

However, for compatibility, setting the application registry is likely a good idea (or at least a good option). I'm not sure whether it's better to

  • always set MetPy's registry as the application registry
  • only set it if it hasn't been set yet (although, such a check against pint._DEFAULT_REGISTRY would require a change in Pint if we didn't want to rely on non-public API)
  • never automatically set it, but include an example in the documentation of how to do so (and demonstrate using it)

jthielen avatar Apr 15 '20 00:04 jthielen

Regardless of interacting with the application/default, what we definitely need is an API that allows a user to set a registry to use (and optionally makes the necessary modifications). I'm not sure how well just swapping out metpy.units.units would work right now...

dopplershift avatar Oct 07 '20 16:10 dopplershift

Ok, so I thought this through some more. Here are the things we need the registry for:

  1. Creating our constants with units--done at import time
  2. Creating other values at function execution time (using units.Quantity())
  3. Setting up math options to make temperature easier to deal with
  4. Enabling more UDUNITS-like parsing (for xarray)

(1) will always be tricky. Best option I can think of is to make the constants more dynamic for lookups, which might go along with other items (i.e. #926). (2) The best way to solve this is doing things like type(sbcape)(6000, 'meter'), which is pretty hideous--I'd really prefer a supported API for accomplishing that. (3) & (4) will always require us setting something, somewhere on a registry.

Until we have solutions at the more granular level for (1) and (2), my thinking is we should try to give a hope of interoperability by doing like xarray-contrib/pint-xarray#32 and:

  1. Create setup_registry() for doing our registry modifications
  2. Use get_application_registry() instead of manually creating a UnitRegistry.

I know our modifications are a bit more scary, but they are readily reversed if you don't want either of those features. The benefit is we would interoperate with other projects using Pint a bit more easily.

@jthielen do you have any quick thoughts?

dopplershift avatar Jun 22 '21 22:06 dopplershift

@jthielen do you have any quick thoughts?

That sounds like a good approach to try out! I don't foresee any show-stopping issues at this point, so as long as it is well tested (e.g., starting from both default registry and mocking some external registries with different configurations), hopefully it all can go well.

Perhaps relevant to the discussion is that cf-xarray now has a slightly-tweaked copy of MetPy's pint registry (https://github.com/xarray-contrib/cf-xarray/pull/197). It would be good to test how this registry plays along with MetPy with this get_application_registry()/setup_registry() process. I'm particularly curious if force_ndarray_like=True breaks anything in MetPy or not and how the future setup_registry() would recognize when it needs to apply any given modification (given that cf-xarray's will already have most if not all of them already in place).

jthielen avatar Jun 22 '21 23:06 jthielen

Was just going to comment that I'd vote to punt this from 1.1 since there are some upstream discussions/proposed changes ongoing that could alter how this is done...but @dopplershift already re-milestoned this 16 hours ago!

jthielen avatar Aug 05 '21 13:08 jthielen