xarray-contrib / pint-xarray

Interface for using pint with xarray, providing convenience accessors
https://pint-xarray.readthedocs.io/en/latest/
Apache License 2.0
105 stars 12 forks source link

rethink the mechanism of the application registry #129

Closed keewis closed 3 years ago

keewis commented 3 years ago

Right now, pint-xarray will call pint.get_application_registry() once and assign a name to the result. This requires the import of libraries modifying the application registry before libraries that use it (like pint-xarray).

One way to fix this would be to always call get_application_registry() when using it, but that may cause other issues...

keewis commented 3 years ago

this could be fixed if pint creates a ApplicationRegistry object that wraps a UnitRegistry instance. get_application_registry would return that, but set_application_registry would instead update the wrapped registry.

Edit: this would also allow us to bypass get_application_registry and instead expose application_registry as a module-global.

The only disadvantage I can think of is that existing type checks would break.

Edit2: if backwards compatibility is a concern, we could expose the module-global and add a set_registry to the ApplicationRegistry object instead of touching get/set_application_registry.

cc @dcherian @jthielen @TomNicholas

keewis commented 3 years ago

that class could be something as simple as

class ApplicationRegistry:
    def __init__(self, registry):
        self._registry = registry

    def set(self, new):
        self._registry = new

    def __getattr__(self, name):
        return getattr(self._registry, name)

    def __dir__(self):
        return dir(self._registry)

(plus a nice repr, maybe)

Edit: it seems neither __getattr__ nor __getattribute__ allow forwarding dunder methods (e.g. __getitem__) so I guess we need to manually forward those

keewis commented 3 years ago

I just realized that it would probably be better to discuss this on pint's issue tracker. I'll propose this there.

keewis commented 3 years ago

hgrecco/pint#1366 has been merged so this can be closed.

I still need to write documentation for it, though...