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

PyLong_AsLong in convert.pxd #171

Merged
merged 1 commit into from
Jan 7, 2025
Merged

PyLong_AsLong in convert.pxd #171

merged 1 commit into from
Jan 7, 2025

Conversation

fchapoton
Copy link
Contributor

see #169

not sure if this is correct. One could maybe remove the conversion ?

@dimpase dimpase requested a review from tornaria January 7, 2025 00:01
@tornaria
Copy link
Contributor

tornaria commented Jan 7, 2025

The change is ok; cython 3.0 has a fallback in the form of

#define PyInt_AsLong                 PyLong_AsLong

and this will be removed in cython 3.1 afaict.

OTOH, I wonder if this PyInt_AS_GEN() function should be deprecated (and remove its use in cypari2/convert.pyx which seems to me dead code; I think isinstance(x, long) is equivalent to isinstance(x, int) in python 3).

It might be worthwhile to try building and testing with a trunk version of cython (what will be 3.1 I guess).

@tornaria
Copy link
Contributor

tornaria commented Jan 7, 2025

As a matter of fact, the current implementation of PyInt_AS_GEN is "broken" in the sense that there is no error checking. Which should be fine for a python 2 int, but may break for a python 3 int. Using PyLong_AsLong does error checking so the new version is better.

@dimpase dimpase merged commit 42ad316 into master Jan 7, 2025
26 of 125 checks passed
@fchapoton fchapoton deleted the fchapoton-patch-1 branch January 7, 2025 07:29
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.

3 participants