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

Expected truthiness of quantities with offsets #866

Closed
jthielen opened this issue Sep 2, 2019 · 2 comments · Fixed by #965
Closed

Expected truthiness of quantities with offsets #866

jthielen opened this issue Sep 2, 2019 · 2 comments · Fixed by #965

Comments

@jthielen
Copy link
Contributor

jthielen commented Sep 2, 2019

With #764 and pydata/xarray#3238, in trying to implement proper np.any and np.all for Quantities, I got to wondering what the expected "truthiness" of a scalar quantity with offset should be. Right now, there seems to be no explicit behavior defined, so it falls back to the magnitude. This leads to the following type of result:

import pint

ureg = pint.UnitRegistry()

temp = ureg.Quantity(0, ureg.degC)

print(bool(temp))
print(bool(temp.to(ureg.degF)))
print(bool(temp.to(ureg.K)))
False
True
True

Should this be the expected behavior?

At first glance, this kind of changing the truthiness of a quantity by changing the unit it is expressed in is a bit unsettling...making a small change like converting the unit doesn't seem like it should change the truthiness. In this case, I think there would need to be an explicit __bool__ method that either converts to the non-offset equivalent or raises a ValueError due to ambiguity.

However, I would also understand if the current behavior is what is expected, since thinking about it more, I would imagine that in most use cases checking for truthiness of a quantity reduces to checking for nonzero values. Also, I'm not sure what pint's stance on breaking existing but undocumented behavior is.

Either way, however, I think tests should be written for the expected behavior and it should be documented clearly (https://pint.readthedocs.io/en/0.9/nonmult.html). Once the expected behavior is decided upon, I'd be glad to put in a PR for that.

@cpascual
Copy link
Contributor

cpascual commented Sep 3, 2019

IMHO both proposed solutions are good (and either would be preferable to the current situation).

Perhaps I would be inclined to raising ValueError, because then the user can always decide to explicitly convert to the non-offset equivalent and re-check.

@jthielen
Copy link
Contributor Author

jthielen commented Sep 9, 2019

Perhaps I would be inclined to raising ValueError, because then the user can always decide to explicitly convert to the non-offset equivalent and re-check.

That sounds like a good way forward, thanks! After looking to implement this, however, I'm now thinking it would make the most sense to wait until #764 (at least the scalar/sequence quantity split) is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants