Skip to content

bpo-39425: Document list.count() corner case #18130

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

Closed
wants to merge 2 commits into from
Closed

bpo-39425: Document list.count() corner case #18130

wants to merge 2 commits into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jan 22, 2020

@vstinner
Copy link
Member Author

cc @corona10

@vstinner
Copy link
Member Author

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
Copy link
Member

Choose a reason for hiding this comment

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

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
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
Copy link
Member Author

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
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
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
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().

@tim-one
Copy link
Member

tim-one commented Jan 24, 2020

I brought this up on python-dev:

https://mail.python.org/archives/list/python-dev@python.org/thread/3ZAMS473HGHSI64XB3UV4XBICTG2DKVF/

and I endorse Guido's reply (very briefly, that it should be documented in a general way, but that it's implementation-defined).

@vstinner
Copy link
Member Author

There is a consensus to not document it in list.count() documentation, so I close my PR.

@vstinner vstinner closed this Jan 25, 2020
@vstinner vstinner deleted the list_count_doc branch January 25, 2020 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants