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

np.isfinite() changes internal type? #481

Closed
dopplershift opened this issue Feb 28, 2017 · 9 comments · Fixed by #957
Closed

np.isfinite() changes internal type? #481

dopplershift opened this issue Feb 28, 2017 · 9 comments · Fixed by #957
Labels
numpy Numpy related bug/enhancement

Comments

@dopplershift
Copy link
Contributor

I have no idea what's going on here:

import numpy as np
from pint import UnitRegistry
ureg = UnitRegistry()
a = np.arange(2.)
au = a * ureg.m

# Numpy Arrays
print('Numpy (base):', [type(v) for v in a])
print('Numpy (isfinite):', [type(v) for v in a if np.isfinite(v)])

# Quantity
print('Pint (base):', [type(v.magnitude) for v in au]) # ok
print('Pint (isfinite):', [type(v.magnitude) for v in au if np.isfinite(v)]) # Huh?

Outputs:

Numpy (base): [<class 'numpy.float64'>, <class 'numpy.float64'>]
Numpy (isfinite): [<class 'numpy.float64'>, <class 'numpy.float64'>]
Pint (base): [<class 'numpy.float64'>, <class 'numpy.float64'>]
Pint (isfinite): [<class 'numpy.ndarray'>, <class 'numpy.ndarray'>]

Why on earth does calling a numpy function modify the Quantity in-place???

The reason this even matters is because this works:

print([format(v, '.0f') for v in au])
['0 meter', '1 meter']

while this:

print([format(v, '.0f') for v in au if np.isfinite(v)])

gives me a traceback (due to NumPy not implementing __format__):

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-64-929696747b8d> in <module>()
----> 1 [format(v, '.0f') for v in au if np.isfinite(v)]

<ipython-input-64-929696747b8d> in <listcomp>(.0)
----> 1 [format(v, '.0f') for v in au if np.isfinite(v)]

/Users/rmay/miniconda3/envs/py35/lib/python3.5/site-packages/pint/quantity.py in __format__(self, spec)
    139             obj = self
    140         return '{0} {1}'.format(
--> 141             format(obj.magnitude, remove_custom_flags(spec)),
    142             format(obj.units, spec)).replace('\n', '')
    143 

TypeError: non-empty format string passed to object.__format__
@gerritholl
Copy link
Contributor

gerritholl commented Mar 2, 2017

Related to #479 . Probably the same cause; set a breakpoint at _Quantity.__getattr__ to verify.

@hgrecco
Copy link
Owner

hgrecco commented Mar 31, 2017

I think you are right, this was done to enable numpy operations working transparently on Pint Quantities. The problem is, I think, that we do know who is asking for that attribute.

Everything is fine if:

  • the semantics do not depend on the dimensionality.
  • the function works on both arrays and python numbers
    (this is the case for numpy.isfinite)

However if dimensionality is importante (like numpy.cos) then we need to cast in place in order to work properly.

I guess that if we remove the inplace conversion then numpy.cos will fail on quantities with python numbers as magnitude.

@hgrecco
Copy link
Owner

hgrecco commented Dec 3, 2019

Revisit after #905

@keewis
Copy link
Contributor

keewis commented Dec 22, 2019

I can't reproduce this on a current master so #905 probably fixed this. We could wait on #925 before we close this, though.

@hgrecco
Copy link
Owner

hgrecco commented Dec 23, 2019

We could wait on #925 before we close this, though.

I would rather add a test and close this issue. A test will always make us remember

@keewis
Copy link
Contributor

keewis commented Dec 23, 2019

I would rather add a test and close this issue. A test will always make us remember

... which is what #925 is about. I only realized this after posting, though

@hgrecco
Copy link
Owner

hgrecco commented Dec 24, 2019

:-D Indeed. I am closing this as I am trying to keep the Issue Tracker free of polution (which good but not perfect yet results)

@jthielen
Copy link
Contributor

:-D Indeed. I am closing this as I am trying to keep the Issue Tracker free of polution (which good but not perfect yet results)

With that in mind, would you also want to close #509, #622, and #678, which are all in a similar position to this issue?

@hgrecco
Copy link
Owner

hgrecco commented Dec 24, 2019

Thanks for pointing this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
numpy Numpy related bug/enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants