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

Add get_application_registry (plus prettify) #738

Closed
wants to merge 2 commits into from

Conversation

Silmathoron
Copy link

@Silmathoron Silmathoron commented Nov 23, 2018

This PR adresses #716 and helps solve #725.

I also moved the __repr__ to PrettyIPython so as not to shadow the pretty representation.

Because the doc uses the tuples with pickle, I also added a warning when making tuples, though I don't understand why you do not advocate for direct pickling...
Checks are made in UnitsContainer to avoid issues with multiple inheritance hierarchies in quantities and units

pint/__init__.py Outdated
"""
Get the application registry which is used for unpickling operations.
"""
assert isinstance(registry, UnitRegistry)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe I am missing something, but... where is the registry variable defined?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously you're not and I was so focused on the pickle part that I did not even check the get_app_registry because it looked "so simple to do", and forgot to delete this line...

@Silmathoron
Copy link
Author

Silmathoron commented Nov 23, 2018

I'm having trouble understanding the check that fails with travis... could you help me out?

EDIT: by the way, do you want me to add some Travis checks? Given what I just did before, it would probably be a good idea to check that the function executes correctly...

@iankharris
Copy link

While we're at it, why don't we update the docs to mention the pint.set_application_registry function? It's by far the easiest way pint to correctly unpickle pint units (in my opinion). It seems like such a trivial change that it's almost a shame to make a pull-request for this, but I really wish the documentation had alluded to this function before I trying to make the to_tuple method described in the docs to work with dask (it doesn't).

@Silmathoron
Copy link
Author

Ok, I'll try to update the doc at some point.
It's going to take me some time to correct the pandas errors because of this... which I think should be changed to >= '0.24' anyway.

However, I still don't know why this call to data causes the pytest error on L967... and I don't think this has anything to do with my PR.

@Silmathoron
Copy link
Author

Hey, there!
Now that pandas 0.24 is out, I thought I would check again what's going on, but it seems the notebook is not working with 0.24... I get some nasty accessors and delegated objects... can anyone tell me what's the status of the pandas support?

@hgrecco
Copy link
Owner

hgrecco commented Dec 21, 2019

Closing this due to #880

@hgrecco hgrecco closed this Dec 21, 2019
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.

4 participants