classification
Title: Add pickle support of Mapping views
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: gvanrossum Nosy List: alexandre.vassalotti, gvanrossum, pitrou, rhettinger, serhiy.storchaka, stutzbach
Priority: normal Keywords: patch

Created on 2015-02-06 09:58 by serhiy.storchaka, last changed 2016-06-26 22:03 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
pickle_mapping_views.patch serhiy.storchaka, 2015-02-06 09:59 review
Messages (17)
msg235475 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-06 09:58
Proposed patch adds pickle support of general Mapping views. See also issue23264.
msg235477 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-06 10:59
There are many ways to pickle e.g. Mapping keys:

1) Default implementation (as an instances of Python class MappingKey). This is implementation depended and exposes private variable _mapping. Changing implementation of the keys() method will break compatibility.

2) As `self.__class__, (self._mapping,)`. It depends on implementation of keys() as MappingKey or MappingKey subclass. It doesn't very subclass friendly if keys() is more complex or MappingKey subclass requires additional parameters.

3) As `self._mapping.__class__.keys, (self._mapping,)`. Implementation agnostic, perhaps more efficient than option 4 because self._mapping.__class__.keys may be shared. But needs protocol 4.

4) As `self._mapping.keys, ()`. Implementation agnostic.

pickle_mapping_views.patch implements option 4, the patch in issue23264 implements option 3 (it works for dict with all protocols, but needs protocol 4 for dict subclasses). Perhaps implementations should be unified (use option 3 with protocol 4 and option 3 otherwise).
msg235493 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-06 18:57
Oh, and yet one option:

5) As a list or a tuple. `copyreg.__newobj__, (list,), None, iter(self._mapping.keys())` or `copyreg.__newobj__, (tuple, tuple(self._mapping))`.
msg267256 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-06-04 15:33
Guido, is it your intention that the ABCs grow pickle support?  

I presume if this were implemented that it would become a requirement for all classes (and their subclasses) that purport to be mappings.  I thought that you had designed to the ABCs to stay focused only on the essential characteristics of mappings.
msg267279 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-04 17:16
The option 3 is now available with pickle protocols >= 2 (but less efficient with protocols < 4).

Adding pickle support of Mapping views doesn't add a requirement for all classes that purport to be mappings.  But if you implement pickle support of your mapping, its views become automatically pickleable.
msg267294 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-06-04 19:10
Raymond, what is your opinion?
msg267297 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-06-04 19:14
Serhiy, why would you want to separately pickle such a view? It's a trivial wrapper around a dict/mapping.

What's the use case?
msg267317 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-04 20:55
Because I can! I just want to ensure that all basic types (for which it makes sense) are correctly and portable pickleable, especially collections and related classes. Note that currently Mapping views are pickleable (as most pure Python classes), but pickle exposes implementation detail (the _mapping attribute). For example pickled keys view of Python implementation of OrderedDict is not fully compatible with C implementation. I want to provide implementation agnostic and more efficient pickling.

>>> keys = collections.abc.KeysView({1: 2, 3: 4})
>>> keys.__reduce_ex__(3)
(<function __newobj__ at 0xb6f64bb4>, (<class 'collections.abc.KeysView'>,), (None, {'_mapping': {1: 2, 3: 4}}), None, None)

There are many ways to do this (msg235477, msg235493). I hesitate with choosing,
msg267318 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-06-04 21:03
I think it's fine to do this for concrete classes, I just don't think it
should be added to ABCs.

--Guido (mobile)
msg267379 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-05 04:58
In this case we need to implement pickle support separately in OrderedDict and any Mapping subclass.
msg267383 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-06-05 05:12
I find it extremely suspicious that if you pickle the keys of a large object it also pickles that object. These are views, not copied data, for a reason. I also take back that this is okay for concrete dict.

As for your reason (http://bugs.python.org/issue23401#msg267317), that doesn't explain the use case for pickling keys views. If you forbade pickling them would any user code break? I seriously doubt it. Who on earth would want to pickle a keys view on an OrderedDict? It doesn't contain any information other than the underlying object. These views exist as alternate APIs to dicts, not as objects by themselves.
msg267486 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-06-06 00:05
> Raymond, what is your opinion?

Whether and how to pickle should be at the discretion of a concrete class rather than a requirement for being a mapping.  For example, it may not make any sense for a persistent dictionary such as a FileDict or an SQLDict.  Also, my mental model of a mapping view is something that is transparent, a window to the world rather than the world itself.
msg267606 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-07 09:23
Should we forbid pickling MappingView?

And what about copying and deepcopying?
msg267691 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-06-07 15:17
No, it should just be up to the implementation. Though you might add a note
to the docs explaining that it's probably a misguided idea.

On Tue, Jun 7, 2016 at 2:23 AM, Serhiy Storchaka <report@bugs.python.org>
wrote:

>
> Serhiy Storchaka added the comment:
>
> Should we forbid pickling MappingView?
>
> And what about copying and deepcopying?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue23401>
> _______________________________________
>
msg267702 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-07 16:31
OK, then I withdraw my proposition.

Thank you for your attention Raymond and Guido.
msg269316 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-26 21:43
Can this issue be closed?
msg269317 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-06-26 21:44
Please close.
History
Date User Action Args
2016-06-26 22:03:01serhiy.storchakasetstatus: open -> closed
resolution: rejected
stage: patch review -> resolved
2016-06-26 21:44:46gvanrossumsetmessages: + msg269317
2016-06-26 21:43:28serhiy.storchakasetmessages: + msg269316
2016-06-07 16:31:10serhiy.storchakasetmessages: + msg267702
2016-06-07 15:17:52gvanrossumsetmessages: + msg267691
2016-06-07 09:23:09serhiy.storchakasetmessages: + msg267606
2016-06-06 00:05:17rhettingersetmessages: + msg267486
2016-06-05 05:12:54gvanrossumsetmessages: + msg267383
2016-06-05 04:58:12serhiy.storchakasetmessages: + msg267379
2016-06-04 21:03:16gvanrossumsetmessages: + msg267318
2016-06-04 20:55:20serhiy.storchakasetmessages: + msg267317
2016-06-04 19:14:41gvanrossumsetmessages: + msg267297
2016-06-04 19:10:26gvanrossumsetmessages: + msg267294
2016-06-04 17:16:21serhiy.storchakasetmessages: + msg267279
versions: + Python 3.6, - Python 3.5
2016-06-04 15:33:02rhettingersetassignee: gvanrossum

messages: + msg267256
nosy: + gvanrossum
2015-02-06 18:57:34serhiy.storchakasetmessages: + msg235493
2015-02-06 10:59:02serhiy.storchakasetmessages: + msg235477
2015-02-06 09:59:54serhiy.storchakasetfiles: + pickle_mapping_views.patch
keywords: + patch
2015-02-06 09:58:55serhiy.storchakacreate