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

BUG: temperature units comparison #407

Closed
neutrinoceros opened this issue Apr 2, 2023 · 1 comment · Fixed by #412
Closed

BUG: temperature units comparison #407

neutrinoceros opened this issue Apr 2, 2023 · 1 comment · Fixed by #412
Labels
bug Something isn't working

Comments

@neutrinoceros
Copy link
Member

  • unyt version: 2.9.5

Description

I would not expect Kelvin and degree Celsius to compare equal, yet this is the current result of Unit.__eq__

What I Did

import unyt as un
assert un.K != un.degC

raises AssertionError

Note that on the main branch (unyt 3.0, using NEP 18), this is breaking array comparison

import numpy as np
import unyt as un
assert all(np.isclose([1, 2, 3] * un.K, [-272.15, -271.15, -270.15] * un.degC))

I think the correct solution is to include Unit.base_offset in the comparison routine, but I'm not 100% certain.

@neutrinoceros neutrinoceros added the bug Something isn't working label Apr 2, 2023
neutrinoceros added a commit to neutrinoceros/unyt that referenced this issue Apr 2, 2023
neutrinoceros added a commit to neutrinoceros/unyt that referenced this issue Apr 2, 2023
neutrinoceros added a commit to neutrinoceros/unyt that referenced this issue Apr 2, 2023
neutrinoceros added a commit to neutrinoceros/unyt that referenced this issue Apr 8, 2023
@neutrinoceros
Copy link
Member Author

FTR #408 is only part of the solution to this issue.
Another problem with np.isclose and temperature units is that the following line will raise an error for units that have a base_offset

if (bu / au).is_dimensionless:

this is probably simple enough to fix, though to avoid building a chain of co-dependent PRs, I'll wait for #410 and #408 to be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant