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

Consider adding public PyLong_GetSign() function #116560

Closed
skirpichev opened this issue Mar 10, 2024 · 5 comments
Closed

Consider adding public PyLong_GetSign() function #116560

skirpichev opened this issue Mar 10, 2024 · 5 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@skirpichev
Copy link
Member

skirpichev commented Mar 10, 2024

Feature or enhancement

Proposal:

Currently there is no way to determine the sign of the PyLongObject value and CPython extensions use private macroses like _PyLong_IsNegative(): https://github.com/aleaxit/gmpy/blob/eb8dfcbd84abcfcb36b4adcb0d5c6d050731dd75/src/gmpy2_convert_gmp.c#L56

PyLong_Sign() will offer GMP-like API to do this.

This was suggested before: #102471 (comment)

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

@skirpichev skirpichev added the type-feature A feature request or enhancement label Mar 10, 2024
skirpichev added a commit to skirpichev/cpython that referenced this issue Mar 10, 2024
* rename _PyLong_Sign() to PyLong_Sign()
* add argument checking, documentation and tests
* keep _PyLong_Sign() as an alias to PyLong_Sign()
@terryjreedy terryjreedy added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 10, 2024
@serhiy-storchaka
Copy link
Member

I think that PyLong_IsPositive(), PyLong_IsNegative() and PyLong_IsZero() are more useful in practice, but there may also be a smaller need for PyLong_Sign(). I suggest to implement the former functions first, and then look if there are enough use cases left for the latter.

@skirpichev
Copy link
Member Author

I worry only on PyLongObject input (probably as most consumers of API). Why three functions better then one? (Disclaimer: API, produced by C-API WG seems too generic for me; IMHO _PyLong_Sign() interface fits use cases in real world much better.)

vstinner added a commit that referenced this issue Jun 3, 2024
@vstinner vstinner changed the title Consider adding public PyLong_Sign function Consider adding public PyLong_GetSign() function Jun 3, 2024
@vstinner
Copy link
Member

vstinner commented Jun 3, 2024

Implemented in commit 61d3ab3

@vstinner vstinner closed this as completed Jun 3, 2024
mliezun pushed a commit to mliezun/cpython that referenced this issue Jun 3, 2024
barneygale pushed a commit to barneygale/cpython that referenced this issue Jun 5, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
@skirpichev
Copy link
Member Author

@vstinner, if I correctly interpret voting, PyLong_IsPositive/Negative/Zero API got support.

Lets reopen this to track implementation, as it's part of the same API?

@serhiy-storchaka
Copy link
Member

It is better to open a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants