homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Test identity first in membership operation of ItemsView, ValuesView and Sequence in collections.abc
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: josh.r, python-dev, rhettinger, serhiy.storchaka, stutzbach, xiang.zhang
Priority: normal Keywords: patch

Created on 2016年05月03日 06:19 by xiang.zhang, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue26915.patch xiang.zhang, 2016年05月03日 06:20 review
issue26915_v2.patch xiang.zhang, 2016年05月03日 06:24 review
issue26915_s2.patch xiang.zhang, 2016年05月05日 10:29 review
issue26915_v3.patch xiang.zhang, 2016年06月04日 08:33 review
issue26915_v4.patch xiang.zhang, 2016年06月04日 08:42 review
Pull Requests
URL Status Linked Edit
PR 503 merged xiang.zhang, 2017年03月06日 09:14
PR 553 merged xiang.zhang, 2017年03月08日 03:16
PR 703 larry, 2017年03月17日 21:00
Messages (19)
msg264688 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年05月03日 06:19
__contains__ operation in ItemsView, ValuesView and Sequence in collections.abc simply test equality with ==. This introduces inconsistent behaviour with built-in containers when encountering objects like NaN, which does not equal to itself. I asked something about this on core-mentorship, see https://mail.python.org/mailman/private/core-mentorship/2016-April/003543.html.
msg264689 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年05月03日 06:24
Add issue number to test method name.
msg264764 - (view) Author: Josh Rosenberg (josh.r) * (Python triager) Date: 2016年05月03日 20:30
At some point someone really needs to decide if the C layer behavior of performing an identity test before full equality checking is something that should be emulated at the Python layer or not. The current state seems ridiculous, where C containers check identity first simply by using the easier RichCompareBool function, while Python containers have to have the identity-then-equality check rewritten explicitly, which feels like a DRY violation.
Makes it harder for non-CPython implementations too, since they end up either not matching CPython behavior, or writing extra code to match the CPython quirks.
I have nothing against this patch, but between PyObject_RichCompareBool and the various slightly strange behaviors in the argument parsing format codes (which leads to silly workarounds like _check_int_field in #20858), I feel like the Python code base is getting cluttered with hacks to emulate the hacky C layer.
msg264773 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年05月04日 02:34
I agree with you josh. Actually that's what I want to know, consistency. But I don't mention it in my post, so Guido only gives what to do in this case. In this thread, it means does Python code have to keep the invariant mentioned in msg75735?
msg264891 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016年05月05日 08:14
New changeset 1c6cf4010df3 by Raymond Hettinger in branch 'default':
Issue 26915: Add identity checks to the collections ABC __contains__ methods.
https://hg.python.org/cpython/rev/1c6cf4010df3 
msg264892 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016年05月05日 08:17
Xiang, hanks for the patch and for the link to Guido's opinion.
Josh, I concur with your sentiments. In this case the burden is light and is likely to prevent future difficulties by keeping the container invariants intact.
msg264896 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016年05月05日 08:25
Is it worth to add a function in the operator module that tests arguments for identity or equality?
msg264898 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年05月05日 08:50
In my opinion it's not worth. If there is such an operator in stdlib, I would expect more, exposing PyObject_RichCompareBool in Python level, providing a new operator like `===`.
msg264900 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016年05月05日 09:16
What about Sequence.index and Sequence.count?
msg264902 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年05月05日 10:29
Oops, I forgot about them. I think they should. Raymond mentioned count in msg75735. Attach patch to take them in.
msg266151 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年05月23日 14:13
Reopen this since the solution is not complete pointed out by Serhiy and the new patch has already been attached(s2).
msg267106 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年06月03日 15:36
Serhiy, actually the patch(issue26915_s2.patch) I uploaded early has tried to make Sequence.index and Sequence.count consistent. Maybe the issue stage is not right.
msg267124 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016年06月03日 18:09
Oh, sorry, I was absentminded. In general the patch LGTM. But I think that using list subclass for tests is not a good idea. If we make a typo in CustomSequence method name (__contain__, inedx, coumt), the test is still passed. It would be better to use a class that doesn't have any sequence-related methods besides explicitly defined.
msg267247 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年06月04日 08:33
Thanks for review Serhiy and nice thoughts. I make the class inherited directly from Sequence. So if the methods in Sequence are OK, the test class is OK.
msg267248 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年06月04日 08:42
Oh, sorry. Make the __len__ signature wrong. Change it.
msg269877 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年07月06日 10:12
ping
msg271151 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016年07月24日 13:38
Ping again.
msg290275 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017年03月24日 22:43
New changeset 78ad039bcf1a8c494cbc8e18380cc30665869c3e by Xiang Zhang in branch '3.6':
bpo-26915: Test identity first in index() and count() of collections.abc.Sequence (GH-553)
https://github.com/python/cpython/commit/78ad039bcf1a8c494cbc8e18380cc30665869c3e
msg290277 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017年03月24日 22:43
New changeset d5d3249e8a37936d32266fa06ac20017307a1f70 by Xiang Zhang in branch 'master':
bpo-26915: Test identity first in membership operation in index() and count() methods of collections.abc.Sequence (GH-503)
https://github.com/python/cpython/commit/d5d3249e8a37936d32266fa06ac20017307a1f70
History
Date User Action Args
2022年04月11日 14:58:30adminsetgithub: 71102
2022年01月20日 22:53:07iritkatriellinkissue23162 superseder
2017年03月24日 22:43:22xiang.zhangsetmessages: + msg290277
2017年03月24日 22:43:01xiang.zhangsetmessages: + msg290275
2017年03月17日 21:00:35larrysetpull_requests: + pull_request609
2017年03月08日 03:44:58xiang.zhangsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017年03月08日 03:16:07xiang.zhangsetpull_requests: + pull_request454
2017年03月06日 09:14:04xiang.zhangsetpull_requests: + pull_request411
2016年07月24日 13:38:52xiang.zhangsetmessages: + msg271151
2016年07月06日 10:12:32xiang.zhangsetmessages: + msg269877
2016年06月04日 08:42:28xiang.zhangsetfiles: + issue26915_v4.patch

messages: + msg267248
2016年06月04日 08:33:20xiang.zhangsetfiles: + issue26915_v3.patch

messages: + msg267247
2016年06月03日 18:09:43serhiy.storchakasetstage: needs patch -> patch review
2016年06月03日 18:09:34serhiy.storchakasetmessages: + msg267124
2016年06月03日 15:36:34xiang.zhangsetmessages: + msg267106
2016年06月03日 12:09:32serhiy.storchakasetresolution: fixed -> (no value)
stage: patch review -> needs patch
2016年05月24日 07:18:39rhettingersetassignee: rhettinger ->
2016年05月23日 14:13:43xiang.zhangsetstatus: closed -> open

messages: + msg266151
2016年05月05日 10:29:16xiang.zhangsetfiles: + issue26915_s2.patch

messages: + msg264902
2016年05月05日 09:16:50serhiy.storchakasetmessages: + msg264900
2016年05月05日 08:50:24xiang.zhangsetmessages: + msg264898
2016年05月05日 08:25:07serhiy.storchakasetmessages: + msg264896
2016年05月05日 08:17:31rhettingersetstatus: open -> closed
resolution: fixed
messages: + msg264892
2016年05月05日 08:14:17python-devsetnosy: + python-dev
messages: + msg264891
2016年05月04日 02:34:20xiang.zhangsetmessages: + msg264773
2016年05月03日 20:48:39serhiy.storchakasetassignee: rhettinger

nosy: + serhiy.storchaka
2016年05月03日 20:30:22josh.rsetnosy: + josh.r
messages: + msg264764
2016年05月03日 06:24:36xiang.zhangsetfiles: + issue26915_v2.patch

messages: + msg264689
2016年05月03日 06:23:53SilentGhostsetnosy: + rhettinger, stutzbach

type: behavior
stage: patch review
2016年05月03日 06:20:37xiang.zhangsetfiles: + issue26915.patch
keywords: + patch
2016年05月03日 06:19:16xiang.zhangcreate

AltStyle によって変換されたページ (->オリジナル) /