classification
Title: collections.abc.Sequence should not provide __reversed__
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: methane, ncoghlan, rhettinger, stutzbach, terry.reedy
Priority: normal Keywords:

Created on 2012-12-18 12:56 by methane, last changed 2019-04-11 13:10 by methane. This issue is now closed.

Messages (8)
msg177688 - (view) Author: Inada Naoki (methane) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2019-04-11 13:10:39methanesetstatus: open -> closed
versions: + Python 3.8, - Python 3.4
messages: + msg339972

resolution: wont fix
stage: needs patch -> resolved
2012-12-26 02:12:19methanesetmessages: + msg178181
2012-12-23 21:23:53rhettingersetmessages: + msg178017
2012-12-22 09:36:26ncoghlansetmessages: + msg177931
2012-12-22 09:35:21ncoghlansetnosy: + ncoghlan
messages: + msg177930

type: enhancement
stage: needs patch
2012-12-22 02:26:21methanesetmessages: + msg177920
2012-12-22 01:07:45terry.reedysetnosy: + rhettinger, stutzbach, terry.reedy

messages: + msg177914
versions: - Python 3.5
2012-12-18 12:56:01methanecreate