Skip to content

API: Retain arr.base more strictly in np.frombuffer#21632

Merged
charris merged 1 commit intonumpy:mainfrom
seberg:relax-memoryview-wrapping
May 29, 2022
Merged

API: Retain arr.base more strictly in np.frombuffer#21632
charris merged 1 commit intonumpy:mainfrom
seberg:relax-memoryview-wrapping

Conversation

@seberg
Copy link
Copy Markdown
Member

@seberg seberg commented May 29, 2022

Checking the implementation of releasebuffer is the most strict
version for when we can safely assume that wrapping into a
memoryview is not necessary.
In theory, view.obj could also be set to a new object even with
the old buffer-protocol, in practice I don't think that should
happen (the docs read like it should not be used).

So this is the minimal fix to retain old behavior as much as
(safely) possible.

Closes gh-21612

Checking the implementation of `releasebuffer` is the most strict
version for when we can safely assume that wrapping into a
`memoryview` is not necessary.
In theory, `view.obj` could also be set to a new object even with
the old buffer-protocol, in practice I don't think that should
happen (the docs read like it should not be used).

So this is the minimal fix to retain old behavior as much as
(safely) possible.

Closes numpygh-21612
@seberg seberg marked this pull request as ready for review May 29, 2022 17:48
@seberg seberg added the 09 - Backport-Candidate PRs tagged should be backported label May 29, 2022
* has to wrap the original object in a Python `memoryview` which deals
* with the lifetime management for us.
* For backwards compatibility of `arr.base` we try to avoid this when
* possible. (For example, NumPy arrays will never get wrapped here!)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NumPy arrays will never get wrapped here!

I'll take your word for that. But why not?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, we don't currently implement releasebuffer. Maybe one day we will, then the test will fail and we should special case them.

@charris charris merged commit e3f28bd into numpy:main May 29, 2022
@charris
Copy link
Copy Markdown
Member

charris commented May 29, 2022

Thanks Sebastian.

@seberg seberg deleted the relax-memoryview-wrapping branch May 29, 2022 22:15
@carterbox
Copy link
Copy Markdown
Contributor

Will this patch be included in numpy 1.22.5, >=1.23.0?

@charris
Copy link
Copy Markdown
Member

charris commented May 31, 2022

It is in 1.23.0. I'm contemplating making 1.22.5 with the patch. A few more complaints might push me to it, as that version will probably be more used over the next year than 1.23.

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jun 8, 2022
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.

BUG: 1.22.4 breaks frombuffer API compared with 1.22.3

3 participants