| msg264688 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
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) * |
Date: 2016-05-03 06:24 |
| Add issue number to test method name. |
|
|
| msg264764 - (view) |
Author: Josh Rosenberg (josh.r) *  |
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) * |
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 ? |
|
|
| msg264891 - (view) |
Author: Roundup Robot (python-dev) |
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) * |
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) * |
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) * |
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) * |
Date: 2016-05-05 09:16 |
| What about Sequence.index and Sequence.count? |
|
|
| msg264902 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-05-05 10:29 |
| Oops, I forgot about them. I think they should. Raymond mentioned count in . Attach patch to take them in. |
|
|
| msg266151 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
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) * |
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) * |
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) * |
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) * |
Date: 2016-06-04 08:42 |
| Oh, sorry. Make the __len__ signature wrong. Change it. |
|
|
| msg269877 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-07-06 10:12 |
| ping |
|
|
| msg271151 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
Date: 2016-07-24 13:38 |
| Ping again. |
|
|
| msg290275 - (view) |
Author: Xiang Zhang (xiang.zhang) * |
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) * |
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 |
|
|