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

Global Quantity, Unit, and Measurement #880

Merged
merged 17 commits into from
Nov 1, 2019

Conversation

crusaderky
Copy link
Contributor

@crusaderky crusaderky commented Sep 13, 2019

This PR addresses several issues I found myself facing when using pint in my own projects.

  • I found myself systematically needing to reference UnitRegistry, Quantity, and Unit in sphinx docstrings and typing annotations.
  • When prototyping and writing snippet code, I got frequently annoyed by having to typing ureg = pint.UnitRegistry() every time, even when I was perfectly content with defaults_en.txt (as most novice users will).
  • I found myself needing to access pint._APP_REGISTRY when creating a Quantity in a dask.distributed function.

Changes:

  • Define global objects pint.Quantity, pint.Unit, and pint.Measurement. They can be used directly, e.g. pint.Quantity(1, "kg"). They use the global application registry.
  • Fix bug where Measurement objects would be accidentally upcast to Quantity upon pickling
  • All Exception classes are now accessible from the top-level module (also in intersphinx)
  • New function pint.get_application_registry
  • Documentation for using pint.set_application_registry in one's application
  • intersphinx users can now reference :class:`pint.UnitRegistry` instead of the internal implementation module :class:`pint.registry.UnitRegistry`
  • Fixed a wealth of sphinx errors (not all)
  • Unit tests for pickling/unpickling custom units through the global registry

@crusaderky crusaderky mentioned this pull request Sep 13, 2019
@crusaderky crusaderky changed the title WIP: Global Quantity and Unit Global Quantity and Unit Sep 16, 2019
@crusaderky crusaderky changed the title Global Quantity and Unit Global Quantity, Unit, and Measurement Sep 16, 2019
@crusaderky
Copy link
Contributor Author

@hgrecco ready for review and merge

@jthielen jthielen mentioned this pull request Sep 20, 2019
@crusaderky
Copy link
Contributor Author

@hgrecco ping

@hgrecco
Copy link
Owner

hgrecco commented Sep 23, 2019

I would like to test how this feels for a while.

@crusaderky
Copy link
Contributor Author

@hgrecco ETA to either merge or tweak requests?

@crusaderky
Copy link
Contributor Author

@hgrecco poke

@crusaderky
Copy link
Contributor Author

@hgrecco if you don't have time to go through this yourself, could you please nominate deputies? It's been 23 days now...

@jthielen
Copy link
Contributor

@hgrecco Any updates on the status of this PR? It would be great to see this make it in soon.

@hgrecco
Copy link
Owner

hgrecco commented Oct 30, 2019

I have been playing with this for a while:

  1. I like how it feels
  2. I like the implementation

but, it seems to me that a changed is needed in the method/property SharedRegistryObject._Registry. This method is triggered when the _Registry attribute has not been set (i.e. when the Quantity was not created after a UnitRegistry is created). If I am reading everything correctly, there is a chance that the registry the Quantity referes first to a Registry and then to other if the application registry is changed by the user in between. This is not a good idea.

If that is the case, I would suggest that after calling SharedRegistryObject._Registry property once, the value is set as an attribute. I know that user can still change the _Registry attribute (It is Python after all) but it would require the user to write in an explicit manner.

@crusaderky
Copy link
Contributor Author

@hgrecco good points. My latest commits address them.

@hgrecco
Copy link
Owner

hgrecco commented Nov 1, 2019

bors r+

bors bot added a commit that referenced this pull request Nov 1, 2019
880: Global Quantity, Unit, and Measurement r=hgrecco a=crusaderky

This PR addresses several issues I found myself facing when using pint in my own projects.

- I found myself systematically needing to reference UnitRegistry, Quantity, and Unit in sphinx docstrings and typing annotations.
- When prototyping and writing snippet code, I got frequently annoyed by having to typing ``ureg = pint.UnitRegistry()`` every time, even when I was perfectly content with defaults_en.txt (as most novice users will).
-  I found myself needing to access pint._APP_REGISTRY when creating a Quantity in a dask.distributed function.

Changes:
- Define global objects ``pint.Quantity``, ``pint.Unit``, and ``pint.Measurement``. They can be used directly, e.g. ``pint.Quantity(1, "kg")``. They use the global application registry.
- Fix bug where Measurement objects would be accidentally upcast to Quantity upon pickling
- All Exception classes are now accessible from the top-level module (also in intersphinx)
- New function ``pint.get_application_registry``
- Documentation for using ``pint.set_application_registry`` in one's application
- intersphinx users can now reference ``:class:`pint.UnitRegistry` `` instead of the internal implementation module ``:class:`pint.registry.UnitRegistry` ``
- Fixed a wealth of sphinx errors (not all)
- Unit tests for pickling/unpickling custom units through the global registry

Co-authored-by: Guido Imperiale <[email protected]>
Co-authored-by: Guido Imperiale <[email protected]>
Co-authored-by: Unknown <[email protected]>
@hgrecco
Copy link
Owner

hgrecco commented Nov 1, 2019

Thanks @crusaderky for all the work and congratulations!

@bors
Copy link
Contributor

bors bot commented Nov 1, 2019

Build succeeded

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.

3 participants