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.

Sign up for GitHub

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: fix an issue where array functions would raise UnitConsistencyError on unyt arrays using non-default unit registries #463

Merged
merged 2 commits into from
Nov 2, 2023
Merged

BUG: fix an issue where array functions would raise UnitConsistencyError on unyt arrays using non-default unit registries #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8754a5c
TST: add tests for bug 462
neutrinoceros Nov 1, 2023
9dbd8ec
BUG: fix an issue where array functions would raise UnitConsistencyEr…
neutrinoceros Nov 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions unyt/_array_functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ def _validate_units_consistency(objs):
# because it's already a necessary condition for numpy to use our
# custom implementations
units = get_units(objs)
sunits = set(units)
if len(sunits) == 1:
unique_units = set(units)
if len(unique_units) == 1 or all(u.is_dimensionless for u in unique_units):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain a little bit more how this resolves the issue (sorry, it may be because it's late for me)?

Copy link
Member Author

Choose a reason for hiding this comment

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

the problem was that

  • Unit() and Unit(registry=UnitRegistry()) have different hash, so they are considered as 2 separate unique units
  • Unit() (or NULL_UNIT is this context) gets automatically inserted for non-unyt array-like data structures only because we want them to be treated as dimensionless

This condition makes it so all dimensionless units are treated as equal as long as nothing else is received.

return units[0]
else:
raise UnitInconsistencyError(*units)
Expand Down
19 changes: 19 additions & 0 deletions unyt/tests/test_array_functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
UnytError,
)
from unyt.testing import assert_array_equal_units
from unyt.unit_object import Unit
from unyt.unit_registry import UnitRegistry

NUMPY_VERSION = Version(version("numpy"))

Expand Down Expand Up @@ -265,6 +267,23 @@ def test_wrapping_completeness():
assert function in all_funcs


@pytest.mark.parametrize(
"arrays",
[
[np.array([1]), [2] * Unit()],
[np.array([1]), [2] * Unit(registry=UnitRegistry())],
# [[1], [2] * Unit()],
],
)
def test_unit_validation(arrays):
# see https://github.com/yt-project/unyt/issues/462
# numpy.concatenate isn't essential to this test
# what we're really testing is the unit consistency validation
# underneath, but we do so using public API
res = np.concatenate(arrays)
assert res.units.is_dimensionless


def test_array_repr():
arr = [1, 2, 3] * cm
assert re.fullmatch(r"unyt_array\(\[1, 2, 3\], (units=)?'cm'\)", np.array_repr(arr))
Expand Down