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

Create global reference to default registry on import? #7

Closed
TomNicholas opened this issue Apr 9, 2020 · 4 comments · Fixed by #32
Closed

Create global reference to default registry on import? #7

TomNicholas opened this issue Apr 9, 2020 · 4 comments · Fixed by #32

Comments

@TomNicholas
Copy link
Member

I didn't realise that there are functions for getting and setting a global pint registry (see pint/#880).

For xarray integration, the ideal eventual usage in my opinion would be to enable pint-xarray by default when both pint and xarray are imported, and pint-xarray is installed, so you only need do

import pint
import xarray

ds = open_mfdataset(files).quantify()

Not having to do

ureg = pint.UnitRegistry()
ds = open_mfdataset(files).quantify(unit_registry=ureg)

would be nice I think.

But that would mean initialising the default registry either during quantify, or during the module imports. set_application_registry() means that particular users can still change it after imports, and get_application_registry() can be called from within quantify calls.

@keewis
Copy link
Collaborator

keewis commented Aug 31, 2020

It looks like in order to resolve #26 we need to resolve (and document) this first. We currently have

def _get_registry(unit_registry, registry_kwargs):
if unit_registry is None:
if registry_kwargs is None:
registry_kwargs = {}
registry_kwargs.update(force_ndarray=True)
# TODO should this registry object then be stored somewhere global?
unit_registry = pint.UnitRegistry(**registry_kwargs)
return unit_registry
which will return a new registry on every call, unless a unit registry was passed to it. That makes it easy to create e.g. a Dataset with units from different unit registries using multiple calls to quantify; we should make that something that has to be explicitly requested.

In order to do so, we might need to either mutate the current application registry, create a new application registry, or save the registry in a module global (but I'm sure I'm missing a cleaner way). Thoughts?

@keewis keewis mentioned this issue Aug 31, 2020
7 tasks
@keewis
Copy link
Collaborator

keewis commented Sep 2, 2020

thinking about this some more, there are a few cases that might be useful to consider here:

  • ds.pint.quantify(a=ureg.m, b="s"): a, b and existing quantity variables should have the same unit registry unless explicitly stated otherwise, so quantify should go through the passed units and the existing quantity variables to see if there are Unit objects it can take a registry from. Obviously, if there are unit objects with different registries this might fail, and we should probably require people to either pass the desired unit registry (this is required if there are units attributes) or to explicitly pass the unit object.
  • ds.pint.quantify(a="m", b="s") with no existing quantity variables: in this case I think we can just take the application registry while making sure the required flags (e.g. force_ndarray_like) are set, if people don't want us to modify the application registry they have to manually provide a unit registry. Edit: even cleaner would be raising if required flags are not set.
  • ds.pint.quantify(a="m", registry_kwargs={"force_ndarray_like": True, ...}): not sure if we should keep this, manually creating a unit registry and passing it to .quantify or setting it as the application registry is just an additional line.

Thoughts, @TomNicholas, @jthielen?

@jthielen
Copy link
Collaborator

jthielen commented Sep 2, 2020

My thoughts on this would be roughly similar to yours @keewis, with one main difference:

  • if a Unit or Quantity object is directly passed, use its registry. If multiple are passed and they don't have consistent registries, then either warn (if left in an operable, but not ideal state) or raise exception (if this breaks something).
  • if only strings (as arguments or from attrs) are specified, then use the application registry from get_application_registry as suggested. If it isn't valid for use in pint-xarray (due to config settings), then raise exception that Unit objects must be used or the registry manually specified with unit_registry argument.
  • I don't think the registry_kwargs option is a good idea. Specifying the registry directly seems cleaner API-wise, and more importantly, since having multiple registries can create many unexpected headaches, we should avoid the proliferation/creation of additional registries if at all possible. The overarching recommendation should be to use one and only one registry, unless your application requires it and you know what you're doing.

@keewis
Copy link
Collaborator

keewis commented Sep 2, 2020

I definitely agree with point 3. However, while your suggestion in point 2 to raise instead of modifying the application registry is cleaner, it also means that users always have to modify / overwrite the application registry before being able to use pint_xarray:

import pint
import pint_xarray
import xarray as xr

pint.set_application_registry(pint.UnitRegistry(force_ndarray_like=True))
# might also work:
# pint.get_application_registry().force_ndarray_like = True

Not sure what the best way to fix that is, maybe a module-global unit registry (pint_xarray.unit_registry)?

Point 1 is not also quite clear to me, which might be because I don't really understand in which situation one would need multiple registries. We could probably exclude support for multiple registries for now and change that in the future?

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 a pull request may close this issue.

3 participants