Skip to content

Conversation

@vkverma9534
Copy link

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

Screenshot 2025-12-23 105556

Passing after my fix

Screenshot 2025-12-23 105955

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.
@welcome
Copy link

welcome bot commented Dec 23, 2025

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

@github-actions github-actions bot added topic-dask topic-arrays related to flexible array support labels Dec 23, 2025
Copy link
Member

@jsignell jsignell left a 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.

@jsignell jsignell added the needs info Issue reporter has not yet provided key information label Dec 23, 2025
@dcherian
Copy link
Contributor

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 contrib-good-first-issue would be a good place to start (example)

@dcherian dcherian closed this Dec 23, 2025
@vkverma9534
Copy link
Author

vkverma9534 commented Dec 23, 2025

@dcherian @jsignell Thanks for the clarification, and sorry for the confusion here.
I misunderstood the purpose of lazy_array_equiv and wrote a test that didn’t reflect its intended behavior. I see now that the test already passes on main and that there isn’t an issue to fix.

I appreciate the feedback and guidance — I’ll take a closer look at existing issues before opening another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs info Issue reporter has not yet provided key information topic-arrays related to flexible array support topic-dask

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants