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

Conversion to Py_hash_t requires an actual "int" on Python 3 #2752

Closed
jdemeyer opened this issue Dec 9, 2018 · 4 comments
Closed

Conversion to Py_hash_t requires an actual "int" on Python 3 #2752

jdemeyer opened this issue Dec 9, 2018 · 4 comments

Comments

@jdemeyer
Copy link
Contributor

jdemeyer commented Dec 9, 2018

There is something special with the way how Cython treats conversion to Py_hash_t (as opposed to other C integer types). Usually, the appropriate __index__ (or __int__) method is called but this is not the case for Py_hash_t on Python 3: that requires an actual Python int. Also on Python 2, conversion to Py_hash_t works fine in all cases.

More precisely: this works on Python 2 and 3:

def to_int(x):
    return <Py_ssize_t>x

class X(object):
    def __index__(self): return 0

print(to_int(X()))

But this only works on Python 2:

def to_int(x):
    return <Py_hash_t>x

class X(object):
    def __index__(self): return 0

print(to_int(X()))
@jdemeyer jdemeyer changed the title Conversion to Py_hash_t requires an actual "int" Conversion to Py_hash_t requires an actual "int" on Python 3 Dec 9, 2018
robertwb added a commit that referenced this issue Dec 10, 2018
Still requires the more conservative __index__ here rather than a possibly
truncating __int__ because this is used in a context where floating point
values should probably be treated specially.

This fixes Github issue #2752.
@scoder scoder modified the milestones: 3.0, 0.29.3 Dec 14, 2018
@scoder scoder closed this as completed Dec 14, 2018
@jdemeyer
Copy link
Contributor Author

This doesn't seem to be fixed in Cython 0.29.x, despite being added to the 0.29.3 milestone.

@scoder
Copy link
Contributor

scoder commented Jun 12, 2019

It's actually set to the 3.0 milestone.
That said, it doesn't really seem disruptive enough to necessitate waiting for 3.0. I'd be ok with picking it over for 0.29.11, if you think it solves a pressing need.

timokau added a commit to timokau/nixpkgs that referenced this issue Nov 27, 2019
cython/cython#2752 has already been accepted
upstream, is minimally invasive (only adds a feature, shouldn't break
any existing code) and is needed for the sage python3 update. Sage needs
this capability because it defines its own integer type.
FRidh pushed a commit to NixOS/nixpkgs that referenced this issue Dec 1, 2019
cython/cython#2752 has already been accepted
upstream, is minimally invasive (only adds a feature, shouldn't break
any existing code) and is needed for the sage python3 update. Sage needs
this capability because it defines its own integer type.
@mkoeppe
Copy link

mkoeppe commented Nov 7, 2021

it doesn't really seem disruptive enough to necessitate waiting for 3.0. I'd be ok with picking it over for 0.29.11, if you think it solves a pressing need.

Given that 3.0 is still in alpha 3 years later, can we get a backport to 0.29.x please?

scoder pushed a commit that referenced this issue Nov 7, 2021
Still requires the more conservative __index__ here rather than a possibly
truncating __int__ because this is used in a context where floating point
values should probably be treated specially.

This fixes Github issue #2752.
@scoder scoder modified the milestones: 3.0, 0.29.25 Nov 7, 2021
@scoder
Copy link
Contributor

scoder commented Nov 7, 2021

I picked the necessary changes into the 0.29.x branch in 100a53f and 6bfd88f

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

No branches or pull requests

3 participants