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-39425: Document list.count() corner case #18130

Open
wants to merge 2 commits into
base: master
from

Conversation

@vstinner
Copy link
Member

vstinner commented Jan 22, 2020

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 22, 2020

cc @corona10

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 22, 2020

FYI CPython 3.7.2, CPython master branch, PyPy 7.1.1 and PyPy3 7.1.1-beta0 all give the same result:

>>> nan=float("nan"); ([nan]*5).count(nan), nan==nan
(5, False)
@@ -994,6 +994,11 @@ Notes:
without copying any data and with the returned index being relative to
the start of the sequence rather than the start of the slice.

(9)
In CPython, :meth:`tuple.count` and :meth:`list.count` consider that an

This comment has been minimized.

Copy link
@ammaraskar

ammaraskar Jan 22, 2020

Member

I would suggest rephrasing to:

In CPython, tuple.count and list.count consider an element to be equal to x if it is identical to x (if element is x is true) skipping the call to element's __eq__() method.

or maybe even

skipping the result of element == x

for the last part.

@tim-one

This comment has been minimized.

Copy link
Member

tim-one commented Jan 22, 2020

I don't think this is a good idea, at least not on its own. The behavior is all over the place. For example, off the top of my head, in list.remove() and list.index() too. And set membership testing. And using objects as dict keys. Documenting it only for .count() methods leaves a misleading impression that they're somehow special in this respect.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 22, 2020

I don't think this is a good idea, at least not on its own. The behavior is all over the place. For example, off the top of my head, in list.remove() and list.index() too. And set membership testing. And using objects as dict keys. Documenting it only for .count() methods leaves a misleading impression that they're somehow special in this respect.

Can we document it somewhere else in this case?

@tim-one

This comment has been minimized.

Copy link
Member

tim-one commented Jan 22, 2020

Can we document it somewhere else in this case?

For all I know, it already is 😉. Sorry, I'm not intimately familiar with the docs anymore.

Because PyObject_RichCompareBool() very deliberately does this, it's intended that "pointer equality implies object equality and not object inequality" be the ordinary behavior. That's why this behavior is "all over the place". It's really x == y and x != y that show oddball behaviors here.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 23, 2020

For all I know, it already is wink. Sorry, I'm not intimately familiar with the docs anymore.

If it is, I would not say that it's not well documented :-) For example, it's not mentionnned in the __eq__ documentation:
https://docs.python.org/dev/reference/datamodel.html#object.__eq__

Maybe this method documentation would be a better place to describe CPython quirks?

@tim-one

This comment has been minimized.

Copy link
Member

tim-one commented Jan 23, 2020

I suggest taking this to python-dev. It's not at all clear to me that this is just a pile of CPython quirks. For example, whether a NaN can be used as a dict key is something that "should" be defined by the language.

Or if consensus is that this is just a pile of CPython quirks, then they need to be documented as such.

Note that x == y (or, more generally, __eq__ and __ne__) cannot do the same, because "rich comparisons" can return any kind of object at all. It's specific to contexts that call PyObject_RichCompareBool(), which must return True/False. But I don't believe the docs spell out which contexts those are, and there are a lot more than just list.count().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.