Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PersistentList slicing returns a non-persistent list on python3 #112

Closed
pakal opened this issue May 1, 2019 · 6 comments · Fixed by #128
Closed

PersistentList slicing returns a non-persistent list on python3 #112

pakal opened this issue May 1, 2019 · 6 comments · Fixed by #128
Assignees
Labels

Comments

@pakal
Copy link

pakal commented May 1, 2019

Since __setslice__() and the likes have been killed in python3 (in favor of __getitem__(slice(...)), it seems that PersistentList must be updated, else slices return regular lists and the DB gets dangerously filled with non-persistent mutables:

Python 3.7.2 (tags/v3.7.2:9a3ffc0492, Dec 23 2018, 23:09:28) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from persistent.list import PersistentList
>>> a = PersistentList([1, 2,3])
>>> isinstance(a, PersistentList)
True
>>> isinstance(a[1:], PersistentList)
False
@jamadden
Copy link
Member

jamadden commented May 2, 2019

I admit to being surprised by this. Sure enough, on Python 2, slicing a subclass of UserList returns an instance of that type (which I wouldn't have expected):

    def __getslice__(self, i, j):
        i = max(i, 0); j = max(j, 0)
        return self.__class__(self.data[i:j])

Whereas on Python 3 it does not:

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

Apparently the Python 2 behaviour even flip-flopped a few times in early releases: in 2.1, it returned the subclass, in 2.2b2 it returned just list (as Python 3 does).

Given that it has worked this way for 7 years on Python 3, changing it to be like Python 2 would be an observable change in behaviour that people may have come to count on (e.g., isinstance(persistent_list[1:], list) would change). OTOH, of course, it could ease porting from Python 2 for those that relied on the old behaviour. I don't know what the right answer is.

@mgedmin
Copy link
Member

mgedmin commented May 2, 2019

Sounds like a bug to me (one that hasn't been discovered for 7 years because almost nobody is using persistent on Python 3, or at least almost nobody was storing slices back into the DB).

@freddrake
Copy link

freddrake commented May 2, 2019 via email

@d-maurer
Copy link

d-maurer commented May 2, 2019 via email

@pakal
Copy link
Author

pakal commented May 7, 2019

We've long advised against using PersistentList, PersistentDict classes without being very aware that they're stored in a single database object, and I can't recall a single instance of using those intentionally myself.

Yes, but aren't those the most standard ways of storing little sequences and mappings in ZODB (which don't require buckets and the likes) ?

If it's consistent with the behaviour of standard python slices, maybe it'd juste need a big warning in docs, so that people take extra care when slicing persistent objects (and when doing other operations which lose persistence, are there?)

@jamadden jamadden added the bug label Feb 15, 2020
@jamadden jamadden self-assigned this Feb 15, 2020
@jamadden
Copy link
Member

Python 3.7.4 recognized this as a bug and flip-flopped again. However, it wasn't backported to 3.6 or earlier (which we still support).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants