classification
Title: cPickle to pickle conversion in py3k missing methods
Type: enhancement Stage: test needed
Components: Extension Modules Versions: Python 3.2
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: alexandre.vassalotti Nosy List: alexandre.vassalotti, amaury.forgeotdarc, belopolsky, jnoller
Priority: normal Keywords: patch

Created on 2008-07-16 19:01 by jnoller, last changed 2010-06-28 21:16 by alexandre.vassalotti. This issue is now closed.

Files
File name Uploaded Description Edit
add_dispatch_check-0.patch alexandre.vassalotti, 2008-08-04 01:44
Messages (10)
msg69816 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-07-16 19:01
I was attempting the patch for issue3125 to py3k, and in it Amaury 
defines a new ForkingPickler:

from pickle import Pickler
class ForkingPickler(Pickler):
    dispatch = Pickler.dispatch.copy()

This is also related to issue3350 I suspect. However, using the 
pickle.Pickler module under py3k, there is no dispatch() method on the 
class:

Trunk:
Python 2.6b1+ (trunk:65015M, Jul 16 2008, 10:15:51) 
[GCC 4.0.1 (Apple Inc. build 5465)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pickle
>>> dir(pickle.Pickler)
['_BATCHSIZE', '__doc__', '__init__', '__module__', '_batch_appends', 
'_batch_setitems', 'clear_memo', 'dispatch', 'dump', 'get', 'memoize', 
'persistent_id', 'put', 'save', 'save_bool', 'save_dict', 
'save_empty_tuple', 'save_float', 'save_global', 'save_inst', 
'save_int', 'save_list', 'save_long', 'save_none', 'save_pers', 
'save_reduce', 'save_string', 'save_tuple', 'save_unicode']
>>> 

py3k:
Python 3.0b1+ (py3k:65011M, Jul 16 2008, 11:50:11) 
[GCC 4.0.1 (Apple Inc. build 5465)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pickle
>>> dir(Pickler)
['__class__', '__delattr__', '__doc__', '__eq__', '__format__', 
'__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', 
'__le__', '__lt__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', 
'__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', 
'bin', 'clear_memo', 'dump', 'fast', 'memo', 'persistent_id']

I think the fix for 3125 resolves your complaint in 3350, but is the 
lack of the dispatch method intentional?
msg69866 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2008-07-17 02:33
The omission of the dispatch dictionary was sort of intentional. But 
now, I think it would be a good idea to add it to _pickle.Pickler. I 
will write a patch ASAP.
msg70165 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2008-07-23 02:12
Just in case you are wondering why I haven't submitted a patch yet, I 
want to let you know that my home computer is currently broken. So, I 
won't be able to work on this until I get my computer fixed (which 
unfortunately could take a few weeks).
msg70169 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-07-23 06:44
A use case:
http://aspn.activestate.com/ASPN/Cookbook/Python/Recipe/572213
shows how code can use the Pickler.dispatch dict, but also some
Pickler.save_xxx function which should be exposed as well.
msg70676 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2008-08-04 01:44
I got a preliminary patch that adds the dispatch dictionary. However,
the patch leaks and it doesn't expose the save_reduce() method (which is
needed by ForkingPickler).
msg71159 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2008-08-15 04:09
I ran into a few problems while trying to fix this issue. First, does
someone know how to add class attributes on extension types? It sounds
like I will need either some tp_dict hacking or a Pickler subclass.

Second, which methods of Pickler should be made public? I know
save_reduce() is needed, but would it be worthwhile to expose more? In
the recipe Amaury linked (which abuses Pickler IMHO), save_global(),
save_dict(), write() and memoize() are used. Exposing all save_* methods
is out of question for now as none were written to be used standalone.

Third, should Pickler allows pickling support for built-in types (e.g.,
int, dict, tuple, etc) to be overridden? Currently, pickle.py allows it.
However, I am not sure if it is a good idea to copy this "feature" in
_pickle.c.
msg71302 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-08-17 23:50
Maybe I missed something, but the (subclassable) python implementation
of Pickler is still available with another name:

from pickle import _Pickler as Pickler
class ForkingPickler(Pickler):
    dispatch = Pickler.dispatch.copy()
msg71394 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2008-08-18 23:29
Yeah, the old Pickler and Unpickler classes are available by design (to
allow testing both implementation). You could subclass _Pickler as a
temporary fix for this issue.
msg108729 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-26 15:08
Do I understand correctly that the issue is that python Pickler class has dispatch attribute but C Pickler does not?  The add_dispatch_check-0.patch patch does not seem to add class attribute, it adds an instance attribute instead.

I also noticed that there are many more class attributes that only Python implementation has:

>>> for x in dir(pickle._Pickler):
...    if not (x.startswith('_') or hasattr(pickle.Pickler, x)):
...       print(x)
... 
dispatch
get
memoize
put
save
save_bool
save_bytes
save_dict
save_float
save_global
save_list
save_long
save_none
save_pers
save_reduce
save_str
save_tuple


The save_* methods are clearly internal and should probably be renamed to begin with __ unless they are intended to be overridden by Pickler subclases
msg108870 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2010-06-28 21:16
> Do I understand correctly that the issue is that python
> Pickler class has dispatch attribute but C Pickler does
> not?

Yes.

> The add_dispatch_check-0.patch patch does not seem
> to add class attribute, it adds an instance attribute
> instead.

I know. That's why I said it was a preliminary patch. :-) Also, see msg71159 about the class vs instance attribute issue.

There's a lot of code out there that do crazy things with pickle's dispatch dictionary. For now, it is best if we leave the method names alone. If people wants to keep doing their funky stuff with pickle, then we can point them to pickle._Pickler and pickle._Unpickler, which will always point to the Python implementations.

That said, I do not believe it's a good idea to add the dispatch attribute to _pickle anymore. But, I acknowledge there is a need for overriding the pickling mechanism for specific types. We should find a clean way to support this.

I am closing this issue as "won't fix".
History
Date User Action Args
2010-06-28 21:16:40alexandre.vassalottisetstatus: open -> closed
resolution: wont fix
messages: + msg108870
2010-06-26 15:08:50belopolskysetversions: + Python 3.2, - Python 3.0
nosy: + belopolsky

messages: + msg108729

type: enhancement
stage: test needed
2008-08-18 23:29:56alexandre.vassalottisetmessages: + msg71394
2008-08-17 23:50:43amaury.forgeotdarcsetmessages: + msg71302
2008-08-15 04:09:38alexandre.vassalottisetmessages: + msg71159
2008-08-04 01:44:46alexandre.vassalottisetfiles: + add_dispatch_check-0.patch
keywords: + patch
messages: + msg70676
2008-07-23 23:46:39alexandre.vassalottilinkissue3431 superseder
2008-07-23 06:44:40amaury.forgeotdarcsetmessages: + msg70169
2008-07-23 02:12:51alexandre.vassalottisetmessages: + msg70165
2008-07-20 13:44:52amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
2008-07-17 02:33:59alexandre.vassalottisetmessages: + msg69866
2008-07-16 19:01:36jnollercreate