msg235475 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2016-06-04 19:10 |
Raymond, what is your opinion?
|
msg267297 - (view) |
Author: Guido van Rossum (gvanrossum) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2016-06-07 09:23 |
Should we forbid pickling MappingView?
And what about copying and deepcopying?
|
msg267691 - (view) |
Author: Guido van Rossum (gvanrossum) * |
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) * |
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) * |
Date: 2016-06-26 21:43 |
Can this issue be closed?
|
msg269317 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-06-26 21:44 |
Please close.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:12 | admin | set | github: 67590 |
2016-06-26 22:03:01 | serhiy.storchaka | set | status: open -> closed resolution: rejected stage: patch review -> resolved |
2016-06-26 21:44:46 | gvanrossum | set | messages:
+ msg269317 |
2016-06-26 21:43:28 | serhiy.storchaka | set | messages:
+ msg269316 |
2016-06-07 16:31:10 | serhiy.storchaka | set | messages:
+ msg267702 |
2016-06-07 15:17:52 | gvanrossum | set | messages:
+ msg267691 |
2016-06-07 09:23:09 | serhiy.storchaka | set | messages:
+ msg267606 |
2016-06-06 00:05:17 | rhettinger | set | messages:
+ msg267486 |
2016-06-05 05:12:54 | gvanrossum | set | messages:
+ msg267383 |
2016-06-05 04:58:12 | serhiy.storchaka | set | messages:
+ msg267379 |
2016-06-04 21:03:16 | gvanrossum | set | messages:
+ msg267318 |
2016-06-04 20:55:20 | serhiy.storchaka | set | messages:
+ msg267317 |
2016-06-04 19:14:41 | gvanrossum | set | messages:
+ msg267297 |
2016-06-04 19:10:26 | gvanrossum | set | messages:
+ msg267294 |
2016-06-04 17:16:21 | serhiy.storchaka | set | messages:
+ msg267279 versions:
+ Python 3.6, - Python 3.5 |
2016-06-04 15:33:02 | rhettinger | set | assignee: gvanrossum
messages:
+ msg267256 nosy:
+ gvanrossum |
2015-02-06 18:57:34 | serhiy.storchaka | set | messages:
+ msg235493 |
2015-02-06 10:59:02 | serhiy.storchaka | set | messages:
+ msg235477 |
2015-02-06 09:59:54 | serhiy.storchaka | set | files:
+ pickle_mapping_views.patch keywords:
+ patch |
2015-02-06 09:58:55 | serhiy.storchaka | create | |