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

ENH: add unyt.testing.assert_equal_units #196

Closed

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Aug 11, 2021

In yt-project/yt#3463, it became apparent that in some cases numpy.testing.assert_equal wasn't enough to validate that a unyt_array instance wasn't changed (it doesn't validate that units stayed constant)
This patch implements unyt.testing.assert_equal_units in line with unyt.testing.assert_allclose_units.
I also added unyt_array.is_convertible_to as a helper function.

@neutrinoceros neutrinoceros marked this pull request as draft August 11, 2021 09:19
@neutrinoceros
Copy link
Member Author

making this a draft for now. I'd like to carefully inspect that the docstrings (essentially copy-pasted) still make sense, but I have to run.

@neutrinoceros neutrinoceros changed the title FEAT: add unyt.testing_assert_equal_units FEAT: add unyt.testing.assert_equal_units Aug 11, 2021
@neutrinoceros neutrinoceros force-pushed the assert_equal_units branch 3 times, most recently from fe4277b to 4521f6b Compare August 11, 2021 16:04
@neutrinoceros
Copy link
Member Author

While working on this I found an annoying bug that I think comes from numpy where error messages from array comparison functions (like numpy.testing.assert_array_equal) omit the unit when representing the compared array, leaving a confusing error message in a failing test, e.g.,

E       AssertionError:
E       Not equal to tolerance rtol=1e-07, atol=0
E
E       Mismatched elements: 1 / 3 (33.3%)
E       Max absolute difference: 1.
E       Max relative difference: 0.25
E        x: unyt_array([1, 2, 3])
E        y: unyt_array([1.e-05, 2.e-05, 4.e-05])

I'll remove the draft status when I'm able to confirm the bug isn't in unyt or when I have a fix for it.

@neutrinoceros
Copy link
Member Author

fell down a rabbit hole. I'm now pretty convinced that the interface to fix the issue I mentioned above is implemented as part of NEP 18.

@neutrinoceros
Copy link
Member Author

neutrinoceros commented Aug 19, 2021

confirmed that the missing part can be solved by implementing NEP 18. I started doing that in #200. Now I need to study the NEP in greater detail to find out wether the present PR is still relevant in this context (we could probably just "overload" np.testing.assert_array_compare ?).

@neutrinoceros neutrinoceros changed the title FEAT: add unyt.testing.assert_equal_units ENH: add unyt.testing.assert_equal_units Oct 22, 2021
@neutrinoceros
Copy link
Member Author

I believe #200 actually supersedes this, closing.

@neutrinoceros neutrinoceros deleted the assert_equal_units branch October 22, 2021 16:26
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.

1 participant