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

bpo-26701: Add documentation for __trunc__ #6022

Merged
merged 3 commits into from
Mar 10, 2018

Conversation

appeltel
Copy link
Contributor

@appeltel appeltel commented Mar 8, 2018

Add explicit documentation of the __trunc__ method beyond the single mention in the math module.

Place the documentation for this method near to __int__, and explain that there is a fallback to x.__trunc__() if x.__int__() is not available.

https://bugs.python.org/issue26701

Copy link
Contributor

@ncoghlan ncoghlan left a comment

Choose a reason for hiding this comment

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

Thanks for this! Your changes look good as far as __trunc__ goes, but they also make it clear that it's not only the __trunc__ path that isn't currently documented properly, but also the __index__ path.

Would you be amenable to adjusting the PR to cover __index__ as well?

@@ -711,8 +711,11 @@ are always available. They are listed here in alphabetical order.

Return an integer object constructed from a number or string *x*, or return
``0`` if no arguments are given. If *x* is a number, return
:meth:`x.__int__() <object.__int__>`. For floating point numbers, this
truncates towards zero.
:meth:`x.__int__() <object.__int__>`. If *x* defines
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you're updating this paragraph anyway, it makes sense to mention __index__ as well, perhaps by converting to a bulleted list of conversion methods in order of priority: x.__index__(), x.__int__(), x.__trunc__().

Called to implement :meth:`math.trunc`. Should return the value of the
object truncated to a :class:`numbers.Integral` (typically an
:class:`int`). If a class defines :meth:`__trunc__` but not
:meth:`__int__`, then :meth:`__trunc__` is called to implement the
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention __index__ here as well.

@appeltel
Copy link
Contributor Author

appeltel commented Mar 8, 2018

@ncoghlan - yes, I'll update this tonight and incorporate your suggestions.

@appeltel
Copy link
Contributor Author

appeltel commented Mar 9, 2018

@ncoghlan So I did some experimentation to try to ensure that I had the order of fallbacks correct, and after further investigation it seems that you can't call int() on an object that has only defined __index__. I can use it as an index, even call hex() on it, but not int():

>>> class Foo:
...     def __index__(self): return 4
... 
>>> f = Foo()
>>> [x for x in range(7)][f]
4
>>> int(f)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: int() argument must be a string, a bytes-like object or a number, not 'Foo'
>>> int(hex(f), 16)
4

This appears to be odd behavior in that something that is obviously designed to be interpreted as an int for the purposes of indexing and slicing can't be coerced into an int, and it seems backwards relative to the aim of PEP-357.

The source code for PyObject * PyNumber_Long(PyObject *o) also clearly does not check the nb_index slot, only nb_int. I found that I can clumsily add this capability by making an (PyLongObject *)_PyLong_FromNbIndex(PyObject *) function largely copied from (PyLongObject *)_PyLong_FromNbInt(PyObject *) but changing the slot, so I guess it was never there.

Maybe I am misreading the code, could you verify?

@ncoghlan
Copy link
Contributor

ncoghlan commented Mar 9, 2018

@appeltel Huh, very strange. I guess the history there is that all of the objects that __index__ was added to help with already defined __int__, so all they needed was a protocol that allowed them to be used in the places that deliberately don't call the potentially lossy conversion methods.

@ncoghlan
Copy link
Contributor

Since the handling of __index__ clearly isn't as straightforward as I thought, I'm going to go ahead and merge this based on the original scope - thanks!

@ncoghlan ncoghlan merged commit 308eab9 into python:master Mar 10, 2018
@miss-islington
Copy link
Contributor

Thanks @appeltel for the PR, and @ncoghlan for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 10, 2018
`int` fails back to `__trunc__` is `__int__` isn't defined, so cover
that in the docs.
(cherry picked from commit 308eab9)

Co-authored-by: Eric Appelt <eric.appelt@gmail.com>
@bedevere-bot
Copy link

GH-6049 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 10, 2018
`int` fails back to `__trunc__` is `__int__` isn't defined, so cover
that in the docs.
(cherry picked from commit 308eab9)

Co-authored-by: Eric Appelt <eric.appelt@gmail.com>
@bedevere-bot
Copy link

GH-6050 is a backport of this pull request to the 3.6 branch.

jo2y pushed a commit to jo2y/cpython that referenced this pull request Mar 23, 2018
`int` fails back to `__trunc__` is `__int__` isn't defined, so cover
that in the docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants