| msg177688 - (view) |
Author: Inada Naoki (methane) *  |
Date: 2012-12-18 12:56 |
| http://docs.python.org/3.3/reference/datamodel.html#object.__reversed__ > Objects that support the sequence protocol should only provide __reversed__() if they can provide an implementation that is more efficient than the one provided by reversed(). collections.abc.Sequence can't provide more efficient method. It only make `reversed()` slower. |
|
|
| msg177914 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2012-12-22 01:07 |
| I would expect that the default method of reversed() and collections.abc.Sequence.__reverse__ are more or less the same. If so, this should be closed as invalid. But I want to hear from the abc experts. |
|
|
| msg177920 - (view) |
Author: Inada Naoki (methane) * |
Date: 2012-12-22 02:26 |
| I believe that using Sequence ABC as mix-in is recommended when implementing custom sequence. But mixing-in it violates "should only provide __reversed__() if they can provide an implementation that is more efficient than the one provided by reversed()." Defining __reversed__ in Sequence ABC also means that classes doesn't have __reversed__ aren't Sequence. While not implementing it is recommended for most cases. |
|
|
| msg177930 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-12-22 09:35 |
| This sounds like a legitimate complaint to me, as the one in the ABC will be at least marginally slower in CPython since it's written in Python while reversed uses a builtin C implementation. Classing it as an enhancement rather than a behavioural bug though, as what we have now isn't *wrong*, it's just not optimal. |
|
|
| msg177931 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-12-22 09:36 |
| The observation about it incorrectly flagging __reversed__ as an expected method for Sequence objects is also valid. |
|
|
| msg178017 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2012-12-23 21:23 |
| Guido put a number of non-optimal implementations in the ABCs. His goal was to define the interface and to supply a working default implementation (see MutableMapping.clear() for a prime example). In the case of __reversed__(), it is unfortunate that it slows down the default implementation of reverse(). The latter only runs faster because it is in C, not because of any algorithmic issue. FWIW, the same is also true of Sequence.__contains__(). This logic in the ABC is straight-forward but slows down the code as compared to Python's C optimizations which can infer a __contains__ method from a sequence that defines __getitem__(). Given that the "issue" isn't algorithmic and is merely a C vs pure Python issue, I recommend leaving the current code as-is. If someone truly cares about the speed issue, it would be an easy matter to supply a C helper function in the ABCs for speeding-up __reversed__ and __contains__ (for an example of how to do this, see _count_elements() in the collections module). |
|
|
| msg178181 - (view) |
Author: Inada Naoki (methane) * |
Date: 2012-12-26 02:12 |
| __contains__ is required for Container. So there is a clear reason to define it. Additionaly, http://docs.python.org/3.3/reference/datamodel.html#object.__contains__ doesn't discourage to implement slower pure-python method. In case of __reversed__, I can't find a reason to require it and reference discourage it explicitly. |
|
|
| msg339972 - (view) |
Author: Inada Naoki (methane) * |
Date: 2019-04-11 13:10 |
| I close this issue because Reversible ABC was added. It's sad that Sequnce.__reversed__ is just makes reversed() slow without any benefit. But removing it without breaking backward compatibility is not easy for now. |
|
|