-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
BUG : Fix lazy_array_equiv for 0-D NumPy arrays #11053
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
Conversation
This PR fixes an invalid runtime type check that used the PEP 604 union syntax (Dataset | DataArray) inside isinstance(), which is not supported by Python and raises a TypeError. The check is updated to use a tuple of classes instead, ensuring correct detection of xarray Dataset and DataArray inputs while preserving the intended behavior.
|
Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
jsignell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what you are trying to solve here. Is there an existing issue that you meant to link? The test you wrote passes on main.
|
Also this is lazy array equiv which does not look at real data. @vkverma9534 if you are looking to help the project then looking at issues tagged |
|
@dcherian @jsignell Thanks for the clarification, and sorry for the confusion here. I appreciate the feedback and guidance — I’ll take a closer look at existing issues before opening another PR. |
This PR fixes an unhandled edge case where lazy_array_equiv returned None for equal 0-dimensional NumPy arrays. The function now explicitly returns a boolean result for scalar arrays, aligning behavior with its intended semantics.
In addition, I introduced a parametrized test that clearly demonstrates the issue: test snippets show how the function previously failed for 0-D NumPy inputs and how it behaves correctly after this change. This makes the regression explicit and ensures the behavior remains covered going forward.
Failing case with existing code
Passing after my fix