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: at should not force overwrite in Dask when copy=None #135

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

crusaderky
Copy link
Contributor

@crusaderky crusaderky commented Feb 6, 2025

Closes #134

  • When x is a dask array,
  • and idx is a bool mask of the same shape as x,
  • and y is a scalar,
  • and copy=None,

don't perform the unnecessary extra step of writing back to x (which encroaches on dask/dask#11722).

This PR works around a bug of Dask 2025.1.0. Starting from Dask 2025.2.0, this becomes a nice-to-have.

Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

thanks @crusaderky, LGTM! I'll merge after putting out a micro release.

Comment on lines +174 to +176
# Dask's default copy value is True for bool masks,
# even if the arrays are writeable.
expect_copy = not is_writeable_array(x) or library is Backend.DASK
Copy link
Member

Choose a reason for hiding this comment

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

is this likely to remain the case indefinitely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see why we should remove it. Even without Dask <=2025.1.0's bug, this PR removes an unnecessary operation. copy=None means "do whatever is best for the library and the situation...

@lucascolley lucascolley merged commit 9f03a41 into data-apis:main Feb 27, 2025
10 checks passed
@crusaderky crusaderky deleted the gh134 branch February 27, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lazy arrays
Projects
None yet
Development

Successfully merging this pull request may close these issues.

at.set behavior for Dask arrays?
2 participants