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: Fix subarray to object cast ownership details #21925

Merged
merged 2 commits into from Jul 12, 2022

Conversation

seberg
Copy link
Member

@seberg seberg commented Jul 5, 2022

This would be the minimal fix for gh-21911 (and an older issue somewhere). However, it would also set a precedence that subarray to object casts will create copies.
That is likely fine, since those casts usually crashed for a while and in many places a copy is OK. It was also always broken from an ownership perspective.
However, for code that used this and assigned to the result, things would obviously change.

There are three options:

  1. Less-minimal fix, because we want view behavior here (and fix it).
  2. Keep it as it is (it was so unreliable that failures are hopefully minimal, and why would one cast to object to then assign?).
  3. We could tag on a warning, I don't think the warning "flag" is used right now. I.e. print out a UserWarning that this is a copy, but may have been a view whenever the array may be written to. (This may not be quite reliable for buffer users like cython, but not sure.)

@seberg
Copy link
Member Author

seberg commented Jul 5, 2022

The fix that returns views is a bit more involved, because we would need to thread proper ownership to everywhere. That is something we may need anyway eventually for proper support of HPy, but it may be a fair bit of complexity (I am not even sure they actually align when it comes to buffers).

@seberg seberg added the 09 - Backport-Candidate PRs tagged should be backported label Jul 12, 2022
@charris charris merged commit b906132 into numpy:main Jul 12, 2022
@seberg seberg deleted the subarray-object-cast branch July 12, 2022 21:18
@charris
Copy link
Member

charris commented Jul 12, 2022

Thanks Sebastian.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants