UserList.__getitem__ doesn't account for slices

[staticshock]

BPO	27639
Nosy	@rhettinger, @bitdancer, @markshannon, @serhiy-storchaka, @vaultah, @staticshock, @mblahay
PRs	bpo-27639: Slices of UserLists are instances of UserList now. #13150 bpo-27639: slices of UserLists should be instances of UserList #4981 bpo-27639: Correct return type for UserList slicing operation #13169 [3.7] bpo-27639: Correct return type for UserList slicing operation (GH-13169) #13181 [3.7] bpo-27639: Correct return type for UserList slicing operation #13203
Files	patch: A patch to return a new UserList when UserList is sliced

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-05-26.17:36:51.318>
created_at = <Date 2016-07-28.06:15:07.780>
labels = ['3.7', '3.8', 'type-bug', 'library', 'easy']
title = "UserList.__getitem__ doesn't account for slices"
updated_at = <Date 2019-05-26.17:36:51.317>
user = 'https://github.com/staticshock'

bugs.python.org fields:

activity = <Date 2019-05-26.17:36:51.317>
actor = 'SilentGhost'
assignee = 'none'
closed = True
closed_date = <Date 2019-05-26.17:36:51.318>
closer = 'SilentGhost'
components = ['Library (Lib)']
creation = <Date 2016-07-28.06:15:07.780>
creator = 'staticshock'
dependencies = []
files = ['43914']
hgrepos = ['351']
issue_num = 27639
keywords = ['patch', 'easy']
message_count = 20.0
messages = ['271503', '271532', '271615', '341618', '341644', '341645', '341647', '341684', '341725', '341726', '341747', '341833', '341854', '341919', '341980', '342131', '342596', '343451', '343551', '343570']
nosy_count = 7.0
nosy_names = ['rhettinger', 'r.david.murray', 'Mark.Shannon', 'serhiy.storchaka', 'vaultah', 'staticshock', 'mblahay']
pr_nums = ['13150', '4981', '13169', '13181', '13203']
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue27639'
versions = ['Python 3.7', 'Python 3.8']

[staticshock]

The docs for UserList say:

List operations which return a new sequence attempt to create an instance of the actual implementation class.

However, UserList([])[:] returns a list, not a UserList.

[bitdancer]

LGTM, but this is change we should probably only make in a feature release, with a note in the What's New porting section.

[rhettinger]

Take a look at the following code from UserList.py in Python2.7 to get an idea of how this was implemented previously:

    def __getslice__(self, i, j):
        i = max(i, 0); j = max(j, 0)
        return self.__class__(self.data[i:j])
    def __setslice__(self, i, j, other):
        i = max(i, 0); j = max(j, 0)
        if isinstance(other, UserList):
            self.data[i:j] = other.data
        elif isinstance(other, type(self.data)):
            self.data[i:j] = other
        else:
            self.data[i:j] = list(other)
    def __delslice__(self, i, j):
        i = max(i, 0); j = max(j, 0)
        del self.data[i:j]

[mblahay]

Here is a test that more explicitly shows the problem.

>>> from collections import UserList
>>> UserList([0,1,2,3,4,5])[0:2].__class__
<class 'list'>
>>> 

It can clearly be seen here that the return type of the slicing operator is list when it should, in this case, be UserList (or whatever class one is using that is derived from UserList).

[mblahay]

The root cause of this issue seems to be the failure to implement type usage in __getitem__ when the deprecated __getslice__ was removed. This is why slicing worked correctly in 2.7, but not the 3.x versions.

In 3.8, the __getitem__ method is used to create the slice, but here we can see that all it does is pass the task to data, which is of type list and then fails to convert the result to the correct type.

  def __getitem__(self, i): return self.data[i]

Using other methods as examples, the fix should look like this:

  def __getitem__(self, i): return self.__class__(self.data[i])

[mblahay]

It is also worth noting that the definition of UserList moved from Lib/UserList.py in 2.7 to Lib/collections/init.py in 3.x

[mblahay]

Results from a quick unit test on the proposed changes were positive:

>>> from collections import UserList
>>> UserList([0,1,2,3,4,5])[0:2].__class__
<class 'collections.UserList'>

If you compare this result with the one a couple comments above, you can see that the result is no longer a list, but rather of type UserList.

[serhiy-storchaka]

PR 4981 LGTM (except that it would be worth to add the author's name in the NEWS entry). It is sad that it was not reviewed for 2 years and is closed now.

[mblahay]

Thank you for bringing up that PR. My team will review and try to find out why it was closed without a merge.