Skip to content

BUG: Stop using PyBytesObject.ob_shash deprecated in Python 3.11. #21321

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

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

felixxm
Copy link
Contributor

@felixxm felixxm commented Apr 11, 2022

This is unnecessary because a cached hash value should not be computed yet for obj. Moreover PyBytesObject.ob_shash was deprecated in Python 3.11, see python/cpython#91020.

Fixes #21317.

@felixxm felixxm changed the title BUG: Stop using PyBytesObject.ob_shash deprecated in Python 3.11, see #21317. BUG: Stop using PyBytesObject.ob_shash deprecated in Python 3.11, fixes #21317. Apr 11, 2022
@seberg
Copy link
Member

seberg commented Apr 11, 2022

This is unnecessary because a cached hash value should not be computed yet for obj

But doesn't setting the cached value to -1 indicate that the has value is not cached? While a hash value of 0 (the default alloc strategy), would be taken as-is?

Simply removing it seems incorrect, e.g. try something like:

a = np.array([b"12345678901234567890"])[()]
assert hash(a) == hash(b"12345678901234567890")

I suspect that this change will mean we get 0 for hash(a)?

@seberg
Copy link
Member

seberg commented Apr 11, 2022

I am wondering if Python should not add a custom alloc function for bytes that sets the hash to -1 (so long as the fields still exists at all). As for NumPy, assuming Python ensures that the hash is invalidated on new versions, we need a version #if` directive to keep setting it to -1 on older versions.

numpy#21317.

PyBytesObject.ob_shash was deprecated in Python 3.11.
@mattip
Copy link
Member

mattip commented Apr 19, 2022

xref python/cpython#91686 to add a custom tp_alloc for bytes, which makes this PR the right way to move forward.

@mattip
Copy link
Member

mattip commented Apr 20, 2022

The upstream fix to have a tp_alloc was merged. NEP 29 does not require we support python 3.11 on NumPy 1.22, but this seems like a simple fix to backport to 1.22.4. @charris should we backport it or only target 1.23 with this?

@mattip mattip added the 09 - Backport-Candidate PRs tagged should be backported label Apr 21, 2022
@mattip mattip merged commit 1686141 into numpy:main Apr 21, 2022
@mattip
Copy link
Member

mattip commented Apr 21, 2022

Thanks @felixxm. I will let @charris add the correct backport milestone, if any

@felixxm
Copy link
Contributor Author

felixxm commented Apr 21, 2022

@mattip Thanks for merging 🎊

@felixxm felixxm deleted the fix-21317 branch April 21, 2022 10:30
@charris
Copy link
Member

charris commented Apr 21, 2022

Seems pretty harmless to backport, although I suspect it won't matter much as we won't release 1.22.x wheels for 3.11.

@charris charris changed the title BUG: Stop using PyBytesObject.ob_shash deprecated in Python 3.11, fixes #21317. BUG: Stop using PyBytesObject.ob_shash deprecated in Python 3.11. May 5, 2022
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Second contribution
Development

Successfully merging this pull request may close these issues.

BUG: Fix ob_shash deprecation warning on Python 3.11
4 participants