Add public functions PyLong_IsPositive(), PyLong_IsNegative() and PyLong_IsZero()

int PyLong_IsPositive(PyObject *);
int PyLong_IsNegative(PyObject *);
int PyLong_IsZero(PyObject *);

They should be identical to existing private functions _PyLong_IsPositive(), _PyLong_IsNegative() and _PyLong_IsZero(), except that the argument is PyObject * instead of PyLongObject *.

The private functions are much more used in the CPython code than _PyLong_Sign (numbers are not accurate):

26 _PyLong_IsZero
 7 _PyLong_IsPositive
71 _PyLong_IsNegative
 8 _PyLong_Sign

Some of cases that use _PyLong_Sign could use _PyLong_IsNegative etc instead. It is expected that they will be more used in the user code too.

They are faster than PyLong_GetSign because they have less code.

They are also more convenient, because you do not need to introduce a variable for result and can use them in expression. For example:

if (PyLong_IsNegative(obj)) {

vs

int sign;
(void)PyLong_GetSign(obj, &sign);
if (sign < 0) {

What are return values? Especially when the argument is not a Python int?

1 for true, 0 for false.

They should be only used with a Python int, like PyUnicode_IsIdentifier() can be only used with a Python str.

0 for false

You meant for non-int's too, right?

They should be only used with a Python int

Then using PyLongObject seems more logical. If so, I don't see why this is better than single

int PyLong_Sign(PyLongobject *obj);

interface, already rejected in #19

PS:

int sign;
(void)PyLong_GetSign(obj, &sign);
if (sign < 0) {

Some compilers (e.g. recent gcc) also emit warning on the first line.

New public API should take PyObject*, and raise TypeError on bad types. These don't do it. Is there any new reason to make an exception to that guideline? I'd hate to repeat the conversation in #19.

Most of the usage in CPython is in unicodeobject.c implemeting PyLong itself. I don't think that's representative for third-party usage.

The functions quite easy to implement using PyLong_GetSign. Is it really worth adding three extra API functions?

Even excluding Objects/longobject.c, there are much more use cases for these functions than for _PyLong_Sign (note that some of these cases for _PyLong_Sign can use _PyLong_IsNegative).

14 _PyLong_IsZero
 2 _PyLong_IsPositive
24 _PyLong_IsNegative
 6 _PyLong_Sign

They are faster and more convenient. They are used in many different files:

Modules/posixmodule.c
Modules/mathmodule.c
Modules/_tkinter.c
Modules/_decimal/_decimal.c
Objects/typeobject.c
Objects/sliceobject.c
Objects/abstract.c
Objects/rangeobject.c
Python/structmember.c
Python/bytecodes.c
Python/optimizer_symbols.c
Python/ast_opt.c
Python/marshal.c

If you insist, we can make them returning 0 or setting an error and returning -1 if the argument is not a Python int. They will still be fast and convenient.

New public API should take PyObject*, and raise TypeError on bad types. These don't do it. Is there any new reason to make an exception to that guideline? I'd hate to repeat the conversation in #19.

PyLong already has exceptions: PyUnstable_Long_IsCompact() and PyUnstable_Long_CompactValue() parameter type is const PyLongObject* instead of PyObject*.

Maybe PyLong_IsPositive(), PyLong_IsNegative() and PyLong_IsZero() belong to the PyUnstable API instead.

All uses outside of Objects/longobject.c currently require the argument to be cast to PyLongObject *. I am sure this will be the same in the user code.

When we use other pointer type than PyObject * in C, this makes the code less type safe.

Is it really worth adding three extra API functions?

As author of the PyLong_Sign() proposal, I would like to see these functions instead of the implemented PyLong_GetSign().

Original idea was to offer GMP-like interface - as mpz_sgn() - that take single PyLongObject* argument (or PyObject* and not raise errors on int's). That's just as convenient and readable as PyLong_Is* functions (at cost of one function, instead of several), e.g.:

if (PyLong_Sign(obj) < 0) {

But with PyLong_GetSign() - this is impossible.

With API like

/* Returns 1 if the object obj is negative integer, and 0 otherwise.   On failure,
   return -1 with an exception set.  This function always succeeds if obj is
   a PyLongObject or its subtype. */
int PyLong_IsNegative(PyObject obj);

most consumers could use one-liners as in the issue description and not worry about calling PyErr_Occurred().

The functions quite easy to implement using PyLong_GetSign.

That's true. But I worry that all consumers of that API will end in implementing such wrappers (or PyLong_Sign-like one).

The gmpy2 project has just one place, that will call this API and this doesn't matter. But not all C API users so lucky.

Even excluding Objects/longobject.c, there are much more use cases for these functions than for _PyLong_Sign (note that some of these cases for _PyLong_Sign can use _PyLong_IsNegative).

To me it looks, that across the whole CPython codebase - only three calls to the _PyLong_Sign() do make sense: floatobject.c, _pickle.c and sliceobject.c. All could be converted to using _PyLong_Is* without loss in readability and, I think, with minimal impact on performance.

That's true. But I worry that all consumers of that API will end in implementing such wrappers (or PyLong_Sign-like one).
The gmpy2 project has just one place, that will call this API and this doesn't matter. But not all C API users so lucky.

For reference, which/how many other consumers are we talking about? I thought gmpy2 is the main user.

CPython itself uses such functions much more than the sign function.

If the main consumer is CPython, we can use the internal C API, no?

CPython is just an example. Looking at the list of files where they are used you can see that they are usable in many different fields.

As I wrote previously, if you want PyLongObject* argument, I suggest to add functions to the PyUnstable API.

I thought gmpy2 is the main user.

Probably, sage is. gmpy2 is much less popular project.

https://github.com/sagemath/sage/blob/b7dd28d2456307802ce4ddcfc692331744f9b0db/src/sage/libs/gmp/pylong.pyx#L95

CPython itself uses such functions much more than the sign function.

But for CPython sometimes _PyLong_Sign() does make sense, e.g.:
https://github.com/python/cpython/blob/92893fd8dc803ed7cdde55d29d25f84ccb5e3ef0/Modules/_pickle.c#L2127

For external projects, I think that Positive/Zero/Negative predicates will handle all use cases.

Based on that link, it looks like sage uses this in combination with other private API, to import/export int data without conversion. We should add API for that whole use case, not for one part of it.

I still think that projects that need these more than once or twice should implement them using PyLong_GetSign, handling the four possible results in the way that's most appropriate for them.

@zooba

@zooba: Would you mind to vote on these APIs?

Done. I don't mind a soft/docs-only deprecation of GetSign, but I see no reason to remove it.

@encukou

@encukou: You're against adding these 3 functions, but other voters are in favor of the addition. Can you consider changing your vote? Same remark question for the keep or remove PyLong_GetSign() vote.

BTW, the sentence "I still think that projects that need these more than once or twice should implement them using PyLong_GetSign, handling the four possible results in the way that's most appropriate for them." contradicts to vote for removing the PyLong_GetSign().

Looks like I'm outvoted. I'll change my vote on the 3 functions, so I don't block progress. But:

I don't mind a soft/docs-only deprecation of GetSign, but I see no reason to remove it.

Did y'all consider that PyLong_GetSign is new in 3.14? We'd be (soft-)deprecating it in the same release where it's added.

Did y'all consider that PyLong_GetSign is new in 3.14?

No I didn't. If it's not been released, then yeah, just remove it.

A PR for adding PyLong_IsPositive, PyLong_IsNegative, and PyLong_IsZero is up for review:

gh-126061: Add PyLong_IsPositive/Zero/Negative() functions python/cpython#126065

If it's not been released, then yeah, just remove it.

Then we have 3/3 votes for/against removing PyLong_GetSign(). What to do?

I have re-set the vote, and marked Steve, Erlend & me as wanting to remove it.

Does anyone want to keep PyLong_GetSign, even though it was only released in an alpha version?

Implementation was merged to main: python/cpython#126065

What's decision for PyLong_GetSign? It seems more votes down...

I voted to remove PyLong_GetSign(). I don't think that we need 4 functions to get a sign, 3 is already a lot :-)

Navigation Menu

Repository navigation

Add public functions PyLong_IsPositive(), PyLong_IsNegative() and PyLong_IsZero() #29

10 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add public functions PyLong_IsPositive(), PyLong_IsNegative() and PyLong_IsZero() #29

Description

Activity

vstinner commented on Jun 13, 2024

serhiy-storchaka commented on Jun 13, 2024

skirpichev commented on Jun 13, 2024

encukou commented on Jun 14, 2024

serhiy-storchaka commented on Jun 14, 2024

vstinner commented on Jun 14, 2024

serhiy-storchaka commented on Jun 14, 2024

skirpichev commented on Jun 16, 2024

encukou commented on Jul 1, 2024

serhiy-storchaka commented on Jul 1, 2024

vstinner commented on Jul 1, 2024

serhiy-storchaka commented on Jul 1, 2024

vstinner commented on Jul 1, 2024

skirpichev commented on Jul 2, 2024

encukou commented on Jul 2, 2024

10 remaining items

encukou commented on Oct 7, 2024

vstinner commented on Oct 14, 2024

zooba commented on Oct 14, 2024

vstinner commented on Oct 17, 2024

skirpichev commented on Oct 18, 2024

encukou commented on Oct 22, 2024

zooba commented on Oct 23, 2024

erlend-aasland commented on Oct 30, 2024

skirpichev commented on Nov 2, 2024

encukou commented on Nov 4, 2024

skirpichev commented on Nov 13, 2024

skirpichev commented on Nov 19, 2024

vstinner commented on Nov 20, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions