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

Fix asarray(copy=False) #15

Merged
merged 3 commits into from
Mar 8, 2024
Merged

Fix asarray(copy=False) #15

merged 3 commits into from
Mar 8, 2024

Conversation

asmeurer
Copy link
Member

@asmeurer asmeurer commented Mar 5, 2024

For NumPy 2.0, this is implemented directly. For NumPy 1, we emulate it by checking if asarray() creates a copy or not.

This also removes support for the np._CopyMode enum in asarray(), as this is not portable.

@rgommers I can't remember if there was a good reason that we didn't just emulate copy=False in this way originally (instead, we raised NotImplementedError).

For NumPy 2.0, this is implemented directly. For NumPy 1, we emulate it by
checking if asarray() creates a copy or not.

This also removes support for the np._CopyMode enum in asarray(), as this is
not portable.
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @asmeurer, this looks good to me.

I don't think there was any reason to not implement this beyond no one thinking about the option to try with copy=False and then checking if a copy was made afterwards, like you are doing in this PR.

@asmeurer
Copy link
Member Author

asmeurer commented Mar 6, 2024

Maybe it had to do with when obj is not an array. Are there instances where copy=False could fail with the buffer protocol?

@rgommers
Copy link
Member

rgommers commented Mar 6, 2024

Maybe it had to do with when obj is not an array. Are there instances where copy=False could fail with the buffer protocol?

No, neither the buffer protocol nor __array_interface__ have that.

@asmeurer
Copy link
Member Author

asmeurer commented Mar 6, 2024

Searched through the discussion at numpy/numpy#18585 and couldn't find anything. I think I just wasn't really clear at the time that numpy copy=False worked like copy=None.

@asmeurer
Copy link
Member Author

asmeurer commented Mar 7, 2024

OK, I updated this to handle Python built-ins and buffer protocol objects properly, and added some more tests since I don't think the test suite really checks copy that sufficiently.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Looks good, let's get this in. Thanks Aaron.

@rgommers rgommers merged commit 8565b2c into data-apis:main Mar 8, 2024
18 checks passed
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.

2 participants