[with spkg, with positive review] cPickle: adds support for class pickling customization

[nthiery]

Original implementation:

The first patch imports the vanilla cPickle.c and test_cpickle.py from python 2.5.2.p9 as sage.misc.cPickle, and updates accordingly the cPickle imports throughout the sage library.

The second patch makes a very small modification to cPickle to allow for customizing how class are pickled via metaclasses.

Final implementation: adds the second patch to the python spkg

See discussions on:

• ​http://groups.google.com/group/comp.lang.python/browse_thread/thread/66c282afc04aa39c/
• ​http://groups.google.com/group/sage-devel/browse_thread/thread/583048dc7d373d6a/

Thanks to Mike, Burcin, and Carl for suggestions on how to handle this.

[nthiery]

I just rebased the first patch upon 3.4.2 final. Please ignore (or better delete) the old version cPickle-5985-import.patch

[nthiery]

Note: the cPickle imports in dsage will need to be updated as well.

[nthiery]

Updated patch:

• Fixes a trivially failing doctest (don't know why I did not see them earlier)
• Adds some more doctest

[mabshoff]

This patch needs an explicit addition of sage/misc/cPickle.c to MANIFEST.in for -sdist to work. It is also listed to have a positive review in the CategoriesRoadMap, but it isn't on the ticket.

Cheers,

Michael

[robertwb]

Followup at #5986

[nthiery]

Replying to mabshoff:

This patch needs an explicit addition of sage/misc/cPickle.c to MANIFEST.in for -sdist to work.

The updated patch hopefully fixes this (please double check).

It is also listed to have a positive review in the CategoriesRoadMap, but it isn't on the ticket.

Oops, fixed. Someone was supposed to give a positive review, but apparently this has not occurred yet.

[craigcitro]

I've rolled an spkg with the patch by Nicolas incorporated -- it's here:

​http://sage.math.washington.edu/home/craigcitro/four-one/python-2.6.2.p2.spkg

It's not ready to be merged (I need to commit the changes in hg), but I'd like Nicolas to confirm that it still works before I play with it too much. Or, if there's an easy testcase, post that and I'll play with it.

I should have more time to look at this tonight (and give it a review, assuming it works).

[nthiery]

Replying to craigcitro:

I've rolled an spkg with the patch by Nicolas incorporated -- it's here:

​http://sage.math.washington.edu/home/craigcitro/four-one/python-2.6.2.p2.spkg

It's not ready to be merged (I need to commit the changes in hg), but I'd like Nicolas to confirm that it still works before I play with it too much. Or, if there's an easy testcase, post that and I'll play with it.

I should have more time to look at this tonight (and give it a review, assuming it works).

Thanks for working on this!

The patch cPickle-5985-copy_reg_classes-submitted.patch includes a fairly complete testsuite (see the addition to sage/misc/test_cpickle_sage.py)

[nthiery]

Replying to nthiery:

Replying to craigcitro:

I've rolled an spkg with the patch by Nicolas incorporated -- it's here:

​http://sage.math.washington.edu/home/craigcitro/four-one/python-2.6.2.p2.spkg

It's not ready to be merged (I need to commit the changes in hg), but I'd like Nicolas to confirm that it still works before I play with it too much. Or, if there's an easy testcase, post that and I'll play with it.

I should have more time to look at this tonight (and give it a review, assuming it works).

Thanks for working on this!

The patch cPickle-5985-copy_reg_classes-submitted.patch includes a fairly complete testsuite (see the addition to sage/misc/test_cpickle_sage.py)

Oops, this testsuite used type.new(...) which apparently does not work anymore with Python 2.6. I just rewrote the testsuite so that it does not use this feature anymore. See attached patch.

Luckily enough, the real applications of this patch readily did not use this feature anymore!

[nthiery]

Replying to nthiery:

Oops, this testsuite used type.new(...) which apparently does not work anymore with Python 2.6. I just rewrote the testsuite so that it does not use this feature anymore. See attached patch.

For the record:

zephyr-/opt/sage-4.0.2>./sage

---

| Sage Version 4.0.2, Release Date: 2009-06-18 | | Type notebook() for the GUI, and license() for information. |

---

Loading Sage library. Current Mercurial branch is: combinat sage: class metaclass(type): ....: def new(cls): ....: return type.new(cls, "bla", (object,), dict()) ....: sage: metaclass() <class 'main.bla'>

zephyr-~>sage

---

| Sage Version 4.1, Release Date: 2009-07-09 | | Type notebook() for the GUI, and license() for information. |

---

Loading Sage library. Current Mercurial branch is: combinat sage: class metaclass(type): ....: def new(cls): ....: return type.new(cls, "bla", (object,), dict()) ....: sage: metaclass()

---

Traceback (most recent call last):

File "<ipython console>", line 1, in <module>

TypeError?: type.init() takes 1 or 3 arguments

Apparently type.new now calls type.init. And I assume that can only be a change in python, not in Sage.

[craigcitro]

I'm uploading a new python spkg, which is the same as the previous one I posted, but based on the most recent python spkg in Sage. It's here:

​http://sage.math.washington.edu/home/craigcitro/python-2.6.2.p4.spkg

This fixes the issue by patching the files in our python spkg instead of importing and using our own copy of cPickle. It also makes the corresponding changes to pickle.

[nthiery]

Replying to craigcitro:

I'm uploading a new python spkg, which is the same as the previous one I posted, but based on the most recent python spkg in Sage. It's here:

​http://sage.math.washington.edu/home/craigcitro/python-2.6.2.p4.spkg

This fixes the issue by patching the files in our python spkg instead of importing and using our own copy of cPickle. It also makes the corresponding changes to pickle.

Sounds good to me! Positive review, up to someone double checking the new patch I attached which imports the test file from the original version of the patch.

Carl, Craig, Robert, please do it soon! William is ok integrating this in 4.1.2 (see IRC). I set back the release to 4.1.2

For the author / reviewer, I don't know exactly what to do since I wrote the first version, Craig reviewed it, wrote the second version which I reviewed :-) Please set whatever you feel appropriate

[craigcitro]

I'm happy with Nicolas's patch to test the new python spkg -- let's finally get this merged! Yay!

[mhansen]

Apply only this patch, after updating the python spkg linked to below

[mhansen]

I updated the patch to do "import cPickle" instead of importing it from sage.misc. After that, everything passes.