Skip to content

bpo-40792: Make PyNumber_Index() always returning exact int.#20443

Merged
serhiy-storchaka merged 5 commits into
python:masterfrom
serhiy-storchaka:pynumber-index-exact-int
May 28, 2020
Merged

bpo-40792: Make PyNumber_Index() always returning exact int.#20443
serhiy-storchaka merged 5 commits into
python:masterfrom
serhiy-storchaka:pynumber-index-exact-int

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented May 27, 2020

Copy link
Copy Markdown
Member

Comment thread Doc/library/operator.rst Outdated
Return *a* converted to an integer. Equivalent to ``a.__index__()``.

.. versionchanged:: 3.10
The type of the result is always :class:`int`.

@mdickinson mdickinson May 27, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth adding a sentence to explain how this is different from the previous behaviour, since I think it'll come as a surprise to many that this wasn't already true before.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please propose a wording? I have problems with formulating this in English.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe something like:

The result always has exact type :class:int. Previously, the result could have been an instance of a subclass of int.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Comment thread Modules/_io/_iomodule.c
Py_off_t result;
PyObject *runerr;
PyObject *value = PyNumber_Index(item);
PyObject *value = _PyNumber_Index(item);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing something here: from my reading of the code, PyLong_AsOff_t is either PyLong_AsLong or PyLong_AsLongLong, and both of those already call _PyNumber_Index, so it looks as though we don't need this call.

Ah, I think I found what I'm missing: PyLong_AsOff_t could be PyLong_AsSsize_t, which doesn't call _PyNumber_Index. Is that right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is right.

It a separate question whether PyLong_AsSsize_t and others should call __index__ implicitly. It is not simple question, because such change may introduce bugs by making presumably atomic code non-atomic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It a separate question whether PyLong_AsSsize_t and others should call __index__ implicitly.

Agreed. It seems worth looking into, but not in this PR.

@mdickinson mdickinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in principle. (Modulo failing CI tests.)

_PyNumber_Index does feel a bit like a premature optimization. Though since the optimization is already present in the original behaviour of PyNumber_Index, I suppose it would be better described as a delayed pessimization.

I suspect we don't have very many cases where an integer-expecting function needs to be fast when passed an instance of an integer subclass - there just aren't that many interesting integer subclasses around, besides bool. (NumPy no longer has any.) I'd probably discourage use of _PyNumber_Index in new code, for simplicity, but it makes sense to use it as a replacement for PyNumber_Index in existing code.

@serhiy-storchaka

Copy link
Copy Markdown
Member Author

The "i" format unit is traditionally used to convert boolean arguments. Without such optimization we would need to create temporary integer objects 0 and 1 to convert boolean arguments. They are cached, but still.

Other case is using IntEnum. Many integer constants in the stdlib are replaced with IntEnum, but they are parsed as integers in the C code.

@serhiy-storchaka
serhiy-storchaka merged commit 5f4b229 into python:master May 28, 2020
@serhiy-storchaka
serhiy-storchaka deleted the pynumber-index-exact-int branch May 28, 2020 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants