classification
Title: deque.pop(index) is not supported
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: Akuli, Jim.Jewett, ShubhamKJha, Yuan, pablogsal, rhettinger, serhiy.storchaka, stutzbach
Priority: normal Keywords:

Created on 2020-07-27 14:18 by Akuli, last changed 2020-07-28 17:48 by pablogsal. This issue is now closed.

Messages (14)
msg374378 - (view) Author: Akuli (Akuli) Date: 2020-07-27 14:18
The pop method of collections.deque can't be used like deque.pop(index), even though `del deque[index]` or deque.pop() without an argument works. This breaks the Liskov substitution principle because collections.abc.MutableMapping supports the .pop(index) usage. Is this intentional?

related typeshed issue: https://github.com/python/typeshed/issues/4364
msg374380 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-07-27 14:28
deque is not a subclass of MutableMapping, so the Liskov substitution principle is not related here.

deque is not registered as a virtual subclass of MutableMapping and it lacks a number of MutableMapping methods.

>>> import collections.abc
>>> issubclass(collections.deque, collections.abc.MutableMapping)
False
>>> sorted(set(dir(collections.abc.MutableMapping)) - set(dir(collections.deque)))
['_MutableMapping__marker', '__abstractmethods__', '__module__', '__slots__', '_abc_impl', 'get', 'items', 'keys', 'popitem', 'setdefault', 'update', 'values']
msg374381 - (view) Author: Akuli (Akuli) Date: 2020-07-27 14:30
I meant MutableSequence instead of MutableMapping. Oops.
msg374382 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-07-27 14:38
>>> issubclass(collections.deque, collections.abc.MutableSequence)
True
>>> sorted(set(dir(collections.abc.MutableSequence)) - set(dir(collections.deque)))
['__abstractmethods__', '__module__', '__slots__', '_abc_impl']

Well, it is a bug.
msg374384 - (view) Author: Yuan (Yuan) Date: 2020-07-27 14:58
Same status as slicing support from MutableSequence.
msg374386 - (view) Author: Shubham Kumar Jha (ShubhamKJha) * Date: 2020-07-27 15:26
Hi, I want to start contributing to CPython. Can I take up this issue?
msg374415 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-07-27 18:58
> Hi, I want to start contributing to CPython. Can I take up this issue?

Sure, go ahead. Feel free to ping one of us in the PR for review
msg374432 - (view) Author: Jim Jewett (Jim.Jewett) * (Python triager) Date: 2020-07-27 22:32
It may well have been intentional, as deques should normally be mutated only at the ends.  But Raymond did make changes to conform to the ABC, so this should probably be supported too.  Go ahead and include docstrings and/or discouraging it, though, except for i=0 and i=-1
msg374458 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-07-28 03:30
This was an intentional decision.  Deques designed for fast access at the end points. Also, pop() is a core deque operation that needs to be fast.  Altering its signature with an optional argument would slow it down.

Thank you for the suggestion, but we really shouldn't do this.
msg374464 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-07-28 05:14
FWIW, the relationship between a concrete class and an ABC is normative with respect to core capabilities but isn't 100% strict.  Concrete classes can vary in small respects when it makes sense.  For example, the __or__ and __and__ methods for collections.Set will accept any iterable, but the concrete does not.  

We use the Liskov substitution principle as a guide, not as a law.  In a number of cases, we choose practicality-beats-purity and allow subclasses to differ in ways than their parents.  Counter() doesn't support fromkeys().  OrderedDict.popitem() has different signature than dict.popitem().

Hope that insight was helpful.
msg374472 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-07-28 07:11
If deque does not fully support the MutableSequence interface, should not it be un-regitered as MutableSequence? Maybe we need other abstract class which would be parent of MutableSequence and deque?
msg374483 - (view) Author: Akuli (Akuli) Date: 2020-07-28 10:43
I don't think it's very common to write code that needs to work with any MutableSequence but not with any Sequence. I think that's the only situation where missing support for deque.pop(index) is a problem.

Maybe deque should be a Sequence but not a MutableSequence. Or maybe there should be a way to say "MutableSequence does not require support for .pop(index) even though you get it by inheriting from MutableSequence".
msg374493 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-07-28 14:44
> If deque does not fully support the MutableSequence interface,
> should not it be un-regitered as MutableSequence?

You could unregister it, but for such a minor variance, it isn't worth it.  We're not unregistering sets and frozenset from Set even though there are minor variances in the operators. 

The ABCs are quite limited in what they do. The underlying machinery is mostly about determining whether a method is present or not.  Concrete classes are free to override, extend or raise NotImplemented.  For example, a Counter is still a mapping even though fromkeys() raises NotImplemented and update() has different semantics than Mapping.  

Jelle has already adapted TypeShed to match the concrete class, so no actual user issue remains.
msg374514 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-07-28 17:48
I am convinced by Raymond's argument, it seems that with the current state of the ABC classes and semantics the most pragmatical solution is the status quo. It would be a weird if deque is a Sequence but not a MutableSequence because it can clearly be mutated.
History
Date User Action Args
2020-07-28 17:48:46pablogsalsetmessages: + msg374514
2020-07-28 14:44:05rhettingersetmessages: + msg374493
2020-07-28 10:43:25Akulisetmessages: + msg374483
2020-07-28 07:11:50serhiy.storchakasetnosy: + stutzbach
messages: + msg374472
2020-07-28 05:14:44rhettingersetmessages: + msg374464
2020-07-28 03:30:22rhettingersetstatus: open -> closed
versions: + Python 3.10
messages: + msg374458

resolution: rejected
stage: resolved
2020-07-27 22:32:18Jim.Jewettsetnosy: + Jim.Jewett
messages: + msg374432
2020-07-27 18:58:26pablogsalsetnosy: + pablogsal
messages: + msg374415
2020-07-27 15:26:20ShubhamKJhasetnosy: + ShubhamKJha
messages: + msg374386
2020-07-27 14:58:14Yuansetnosy: + Yuan
messages: + msg374384
2020-07-27 14:38:52serhiy.storchakasettype: behavior
messages: + msg374382
2020-07-27 14:30:22Akulisetmessages: + msg374381
2020-07-27 14:28:37serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg374380
2020-07-27 14:26:57xtreaksetnosy: + rhettinger
2020-07-27 14:18:55Akulicreate