Add PyLong_Sign() public function

API: int PyLong_Sign(PyObject *obj, int *sign)

Retrieve the sign of integer object obj (0, -1 or +1 for zero,
negative or positive integer, respectively) in a variable sign.

Return 0 on success, else -1 with an exception set. This function
always succeeds if obj is a :c:type:PyLongObject or its subtype.

PR: python/cpython#116561

I would like to propose adding the API directly to the limited API, what do you think?

@markshannon

IIRC @markshannon has opinions on the API for long ints. Please ask him first.

What's the use case for this?

Why pass in a PyObject *, rather than a PyLongObject *?
int PyLong_Sign(PyLongObject *obj) wouldn't need to worry about errors.

What's the use case for this?

This is something we like to have to support int<->mpz conversion of "big enough" int's with mpz_import/export. See current code in the gmpy2 (similar approach has Sage): for reading and for writing.

Now we have *From/AsNativeBytes functions to do conversion of "big enough" int's, but this is much more slow than above approach.

Together with PyLong_Sign(), we need functions to access absolute value of "big enough" integer as an array of "digits" (for reading or writing). The GMP supports this kind of API with mpz_limbs_read() and mpz_limbs_write() functions. Perhaps, this is a more simple alternative (on CPython side) to PyInt_Import/Export functions.

Edit, a complete interface:

/* reading */
int PyLong_Sign(PyLongObject *obj);  // mpz_sgn()
Py_ssize_t PyLong_DigitCount(PyLongObject *obj);  // mpz_size()
const digit * PyLong_AsDigits(PyLongObject *obj);  // mpz_limbs_read()

/* writing (former _PyLong_FromDigits, used in _decimal.c) */
PyLongObject* PyLong_FromDigits(int negative, Py_ssize_t digit_count, digit *digits);

int PyLong_Sign(PyLongObject *obj) wouldn't need to worry about errors.

This is something I was thinking first. Clearly, it's enough for above use case. If this kind of API is acceptable (so far, I see only PyUnstable_* functions of this type) - I would like to adopt one.

@markshannon

@markshannon:

What's the use case for this?

A code search on PyPI top 5,000 projects (at 2023-11-15) finds usage in 10 projects:

Cython (3.0.5)
cffi (1.16.0)
datatable (1.0.0)
line_profiler (4.1.2)
mariadb (1.1.8)
msgspec (0.18.4)
orjson (3.9.10)
pickle5 (0.0.12)
pyodbc (5.0.1)
zodbpickle (3.1)

Code:

Details

cffi-1.16.0.tar.gz: cffi-1.16.0/src/c/_cffi_backend.c: if (_PyLong_Sign(ob) < 0)
cffi-1.16.0.tar.gz: cffi-1.16.0/src/c/_cffi_backend.c: return _PyLong_Sign(ob) != 0;

Cython-3.0.5.tar.gz: Cython-3.0.5/Cython/Utility/TypeConversion.c: #define __Pyx_PyLong_Sign(x)  (((PyLongObject*)x)->long_value.lv_tag & _PyLong_SIGN_MASK)
Cython-3.0.5.tar.gz: Cython-3.0.5/Cython/Utility/TypeConversion.c: #define __Pyx_PyLong_IsNeg(x)  ((__Pyx_PyLong_Sign(x) & 2) != 0)
Cython-3.0.5.tar.gz: Cython-3.0.5/Cython/Utility/TypeConversion.c: #define __Pyx_PyLong_IsZero(x)  (__Pyx_PyLong_Sign(x) & 1)
Cython-3.0.5.tar.gz: Cython-3.0.5/Cython/Utility/TypeConversion.c: #define __Pyx_PyLong_IsPos(x)  (__Pyx_PyLong_Sign(x) == 0)
Cython-3.0.5.tar.gz: Cython-3.0.5/Cython/Utility/TypeConversion.c: ((1 - (Py_ssize_t) __Pyx_PyLong_Sign(x)) * __Pyx_PyLong_DigitCount(x))
Cython-3.0.5.tar.gz: Cython-3.0.5/Cython/Utility/TypeConversion.c: #define __Pyx_PyLong_CompactValue(x)  ((1 - (Py_ssize_t) __Pyx_PyLong_Sign(x)) * (Py_ssize_t) __Pyx_PyLong_Digits(x)[0])

datatable-1.0.0.tar.gz: datatable-1.0.0/src/core/python/int.cc: int sign = _PyLong_Sign(v);
datatable-1.0.0.tar.gz: datatable-1.0.0/src/core/python/int.cc: int sign = _PyLong_Sign(v);
datatable-1.0.0.tar.gz: datatable-1.0.0/src/core/python/obj.cc: int sign = _PyLong_Sign(v);

line_profiler-4.1.2.tar.gz: line_profiler-4.1.2/line_profiler/python25.pxd: int   _PyLong_Sign (object)  # No error.

mariadb-1.1.8.tar.gz: mariadb-1.1.8/mariadb/mariadb_codecs.c: if (_PyLong_Sign(value->value) < 0)

msgspec-0.18.4.tar.gz: msgspec-0.18.4/msgspec/_core.c: bool neg = _PyLong_Sign(obj) < 0;

pickle5-0.0.12.tar.gz: pickle5-0.0.12/pickle5/_pickle.c: int sign = _PyLong_Sign(obj);

pyodbc-5.0.1.tar.gz: pyodbc-5.0.1/src/connection.cpp: if (_PyLong_Sign(value) >= 0)
pyodbc-5.0.1.tar.gz: pyodbc-5.0.1/src/params.cpp: pNum->sign = _PyLong_Sign(cell) >= 0;

zodbpickle-3.1.tar.gz: zodbpickle-3.1/src/zodbpickle/_pickle_33.c: int sign = _PyLong_Sign(obj);

One usage is to ... test the sign of a number :-) Examples:

bool neg = _PyLong_Sign(obj) < 0;

and

if (_PyLong_Sign(value) >= 0) ...

@markshannon:

Why pass in a PyObject *, rather than a PyLongObject *?

For consistency with other PyLong APIs. Example: long PyLong_AsLong(PyObject *).

Generic PyObject* vs specific type (such as PyLongObject*) was also discussed in the Add PyDict_GetItemRef() function issue. It was decided to stick to PyObject*.

Generic PyObject* vs specific type (such as PyLongObject*) was also discussed in the python/cpython#106004 issue. It was decided to stick to PyObject*.

JFR, it was discussed rather in the pr thread: python/cpython#106005

Perhaps, this is the relevant issue: capi-workgroup/api-evolution#29

By the way, I'm surprised that the C API has no function to compare a Python int object to a C integer, something like:

int PyLong_CompareWithLong(PyObject*, long, int *result)

Returns 0 on success. In that case, *result is set to -1 if less than, 0 if equal, +1 if greater than.
Set an exception and return -1 on error.

I'm mentiong such hypothetical function since _PyLong_Sign(obj) is currently used by a few projects to compare a Python int to zero: is it equal to zero? Smaller than zero? Greater than zero?

It could be done with:

int cmp;
if (PyLong_CompareWithLong(number, 0, &cmp) < 0) ... handle error ...
if (cmp == 0) ... equal to zero
else if (cmp < 0) ... smaller than zero
else ... greater than zero

Another data: I just added Py_GetConstantBorrowed(Py_CONSTANT_ZERO) and Py_GetConstantBorrowed(Py_CONSTANT_ONE) to the C API, and these objects can be used with PyObject_RichCompareBool().

int cmp;
PyObject *zero = Py_GetConstantBorrowed(Py_CONSTANT_ONE);
cmp = PyObject_RichCompareBool(number, zero);
if (cmp < 0 && PyErr_Occurred()) ... handle error ...
if (cmp == 0) ... equal to zero
else if (cmp < 0) ... smaller than zero ...
else ... greater than zero ...

But I dislike this code, since it uses a borrowed reference and it requires to call PyErr_Occurred() :-(

@encukou

@encukou @gvanrossum @iritkatriel @zooba: What's your opinion on adding int PyLong_Sign(PyObject *obj, int *sign) API to Python 3.13?

+1 from me.

There was no formal vote, but most members of the WG decided to use PyObject * rather than concrete types:

All our deliberately public APIs should be PyObject * to minimise abstraction leakage, and do their type checks (raising TypeErrors, not assertions). Fast APIs that skip checks should be the special case, not the default.

For “evolution”, let's stick to that.

Hopefully PyObject_IsTrue is a relatively quick way to compare to zero (assuming you already know you have the right type).

I'm good with adding the function, and I'm about 50/50 between Victor's proposed API and one that returns a "switch"-able result:

switch(PyLong_Sign(o)) {
case PY_LONG_POSITIVE:
case PY_LONG_NEGATIVE:
case PY_LONG_ZERO:
    break;
default:
    // error raised
}

Though honestly, none of the times when I've needed this have I needed to optimise for a sign check before conversion, especially since for compact values it'll cost about the same to assign *sign as *value. But possibly there's a use for checking without converting at all.

In general I find APIs with output variables awkward, so I want to use them only when necessary. Since the sign function has only four possible outcomes (error, negative, zero, positive) I feel it doesn't really warrant the output variable, so I'm supportive of Steve's solution.

In general I find APIs with output variables awkward, so I want to use them only when necessary. Since the sign function has only four possible outcomes (error, negative, zero, positive) I feel it doesn't really warrant the output variable, so I'm supportive of Steve's solution.

I agree, with -1 for error and non-negative values for the three valid outcomes.

I'm fine with any API as soon as it doesn't require to call PyErr_Occurred().

If we go with the enum-like approach, I suggest to use more specific names, add "IS_" in the name, and use Py_ prefix rather than PY_:

#define Py_LONG_IS_ZERO 0
#define Py_LONG_IS_NEGATIVE 1
#define Py_LONG_IS_POSITIVE 2

Usage would be:

int sign = PyLong_Sign(obj);
if (sign < 0) { ... error ...; }
if (sign == Py_LONG_IS_ZERO || sign == Py_LONG_IS_POSITIVE) {
    ... obj >= 0 ...
}
if (sign == Py_LONG_IS_NEGATIVE) {
    ... obj < 0 ...
}

The advantage of int PyLong_Sign(PyObject *obj, int *sign) API is that no constant needs to be defined and sign can be used more naturally:

int sign;
if (PyLong_Sign(obj, &sign) < 0) { ... error ...; }
if (sign >= 0) {
    ... obj >= 0 ...
}
if (sign < 0) {
    ... obj < 0 ...
}

The gmpy project needs to check if a number is negative: https://github.com/aleaxit/gmpy/blob/eb8dfcbd84abcfcb36b4adcb0d5c6d050731dd75/src/gmpy2_convert_gmp.c#L41-L75

int negative = _PyLong_IsNegative(templong);
...
if (negative) {
    mpz_neg(result->z, result->z);
}

So any API would be fine.

How about we define the result as follows?

-2 error
-1 negative
0 zero
1 positive

I think it will be confusing in light of the c api convention of <0 indicating error. That's probably stronger in this case than the convention on comparison result being -1,0,1.

PyObject_RichCompareBool() API doesn't have this issue since it takes two arguments and a 3rd is the comparison operator:

/* Perform a rich comparison with integer result.  This wraps
   PyObject_RichCompare(), returning -1 for error, 0 for false, 1 for true. */
int PyObject_RichCompareBool(PyObject *v, PyObject *w, int op)

If we do provide both, PyLong_SignUnchecked is the one that will get used.
I'd be very surprised if users chose the more complex variant over the simpler one.

If we were to do both, the latter ought to be PyUnstable_Long_Sign (or PyUnstableLong_Sign? Or PyLongUnstable_Sign?) and probably be defined as an inline function (like the existing private function).

Otherwise, 100% agree with Guido (and 90% with Mark: I think the pointer cast will sometimes be less convenient than chaining conditions (if (...Sign(x, &s) && s > 0 && ...NativeBytes(...)), but for the most part the simpler function will win).

I can live with having just the two-argument version if we rename it to PyLong_GetSign.

Sounds great!

PyLong_SignUnchecked is the one that will get used.
I'd be very surprised if users chose the more complex variant over the simpler one.

That's OK -- with a name that announces that this function is unusual.
(And if we add it, I'd consider it inconsistent to not have the “regular” variant.)

PyUnstable_Long_Sign

I don't think we need to worry about API stability here. IMO, the convenient API is inconsistent with how we want new API to look. While consistency and stability are correlated, I'd rather keep them separate (Unchecked vs. Unstable).

@gvanrossum

int PyLong_GetSign(PyObject *obj, int *sign)

On success, set *sign to sign of integer object obj (0, -1
or +1 for zero, negative or positive integer, respectively)
and return 0.

On failure, return -1 with an exception set.
This function always succeeds if obj is a
PyLongObject or its subtype.

Have any of the likely users asked for this two argument form, as opposed to int PyLong_GetSign(PyLongObject *obj)?

It hardly matters, they aren't getting one that accepts PyLongObject as an argument. The choice will be between putting the error code or the result in an output parameter, and previously we've agreed (in a very rare case of agreement 😄 ) that the error code should be the returned value.

@iritkatriel

@iritkatriel, what are your thoughts?

@erlend-aasland

@erlend-aasland: Are you ok with API proposed in #19 (comment) ? Tell me if you need more context/information. And it's ok if you need more time to decide.

I'm fine with the proposed API; I voted.

Thanks all. The SC adopted int PyLong_GetSign(PyObject *obj, int *sign) API: #19 (comment). I close the issue.

@markshannon

I concur with @markshannon. For such low-level function the overhead of checking the type is too high, and it makes the API less convenient.

Also, I think that PyLong_IsPositive(), PyLong_IsNegative() and PyLong_IsZero() are more useful in practice and adds less overhead. So I propose to implement these functions first, and then look if there are enough use cases are left for PyLong_Sign().

These functions are used when performance is very important. When it is not important, you can use PyObject_RichCompareBool().

These functions are used when performance is very important.

In which kind of code the performance of PyLong_GetSign() would matter? I don't expect this function to be called often, like not more than once per integer, and not in "hot code".

PyLong_GetSign() cannot fail if the argument is a Python int. If you know that the parameter is a Python int, you can simply ignore the error handling (maybe using an assertion, just in case).

I do not remember details, it was many years ago, but I used Py_SIZE(x) < 0 instead of _PyLong_Sign(x) < 0 more than once in the past because the difference was noticeable. You can search all occurrences of "_PyLong_Is" and try to replace them with _PyLong_Sign.

BTW, the _PyLong_Is*() functions are used much more (104 occurrences total: 26 _PyLong_IsZero, 7 _PyLong_IsPositive and 71 _PyLong_IsNegative) than _PyLong_Sign() (8). And several of these _PyLong_Sign() calls could be replaced with _PyLong_Is*().

The API proposed here is not only slower, it is less convenient. You need to introduce a variable, and even if you ignore error, you cannot use it in expression.

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

instead of

if (PyLong_IsNegative(obj)) {

Multiply this by 112 use cases.

The API proposed here is not only slower, it is less convenient.

These aspects were taken in account in the decision.

This API takes PyObject*. If you consider that it's way too slow and the API is not convenient, you can propose adding a PyUnstable API which takes PyLongObject*. I'm not convinced that it's needed.

Also, this issue is not closed, so I suggest to open a new one.

Navigation Menu

Repository navigation

Add PyLong_Sign() public function #19

15 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add PyLong_Sign() public function #19

Description

Activity

gvanrossum commented on Mar 11, 2024

markshannon commented on Mar 11, 2024

skirpichev commented on Mar 12, 2024

vstinner commented on Mar 15, 2024

skirpichev commented on Mar 20, 2024

vstinner commented on Mar 22, 2024

vstinner commented on Apr 15, 2024

encukou commented on Apr 15, 2024

zooba commented on Apr 15, 2024

gvanrossum commented on Apr 15, 2024

iritkatriel commented on Apr 15, 2024

vstinner commented on Apr 15, 2024

gvanrossum commented on Apr 15, 2024

iritkatriel commented on Apr 15, 2024

vstinner commented on Apr 15, 2024

15 remaining items

markshannon commented on Apr 23, 2024

zooba commented on Apr 23, 2024

encukou commented on Apr 24, 2024

encukou commented on May 1, 2024

markshannon commented on May 1, 2024

zooba commented on May 1, 2024

encukou commented on May 6, 2024

vstinner commented on May 31, 2024

erlend-aasland commented on Jun 1, 2024

vstinner commented on Jun 1, 2024

serhiy-storchaka commented on Jun 3, 2024

vstinner commented on Jun 3, 2024

serhiy-storchaka commented on Jun 3, 2024

vstinner commented on Jun 5, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions