Opened 10 years ago
Closed 10 years ago
#5986 closed defect (fixed)
[with patch, positive review] Workaround mishandled nested classes in Python and cPickle
| Reported by: | nthiery | Owned by: | nthiery |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.2 |
| Component: | misc | Keywords: | pickling, nested classes |
| Cc: | sage-combinat, cwitty | Merged in: | sage-4.2.alpha0 |
| Authors: | Craig Citro, Nicolas M. Thiéry | Reviewers: | Robert Bradshaw |
| Report Upstream: | Work issues: | ||
| Branch: | Commit: | ||
| Dependencies: | Stopgaps: |
Description (last modified by )
With the python code below::
class A:
class B:
pass
Python 2.6 erroneously set the B.name to "B" instead of "A.B".
Furthermore, upon pickling (here in save_global) *and* unpickling (in load_global) a class with name "A.B" in a module mod, the standard cPickle module searches for "A.B" in mod.dict instead of looking up "A" and then "B" in the result.
This patch works around this by a patch to cPickle.c (in fact a copy of it in the Sage source tree; see #5985) which fixes the name for B to its appropriate value A.B, and inserts 'A.B' = A.B in mod.dict (hacky, but seems to work) the first time A.B is pickled, and fixes load_global to implement a proper lookup upon unpickling.
It also ensures that sage/interfaces/sage0.py uses loads/dumps from sage_object rather than calling directly cPickle.loads/dumps (+1 by cwitty on this change)
Python source experts are more than welcome to rework/rewrite this patch!
Attachments (2)
Change History (11)
comment:1 Changed 10 years ago by
- Cc cwitty added
- Description modified (diff)
Changed 10 years ago by
comment:2 Changed 10 years ago by
- Description modified (diff)
comment:3 Changed 10 years ago by
- Status changed from new to assigned
comment:4 follow-up: ↓ 5 Changed 10 years ago by
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 10 years ago by
comment:6 in reply to: ↑ 5 Changed 10 years ago by
Replying to jason:
Replying to robertwb:
This workaround it a bit to hackish for my taste, but it's been implemented and tested. Followup at #6121.
Does that mean that this ticket can be closed?
Not before Robert or someone else writes a proof of concept patch upon the category code proving that #6121 is a usable replacement for this one to get pickling to work for parents and categories. See discussion on Sage devel.
comment:7 Changed 10 years ago by
- Milestone changed from sage-4.1.3 to sage-4.1.2
- Reviewers set to Robert Bradshaw
comment:8 Changed 10 years ago by
- Status changed from needs_review to positive_review
- Summary changed from [with patch, needs review] Workaround mishandled nested classes in Python and cPickle to [with patch, positive review] Workaround mishandled nested classes in Python and cPickle
Much less intrusive--too bad we didn't pursue this just a bit more back in June.
comment:9 Changed 10 years ago by
- Merged in set to sage-4.2.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
This workaround it a bit to hackish for my taste, but it's been implemented and tested. Followup at #6121.