Opened 10 years ago
Closed 10 years ago
#11069 closed enhancement (fixed)
Don't use version of Sage in default pickle directory
Reported by: | jdemeyer | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.1 |
Component: | pickling | Keywords: | pickle |
Cc: | Merged in: | sage-4.7.1.alpha2 | |
Authors: | Jeroen Demeyer | Reviewers: | Nicolas M. Thiéry |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
I don't see any good reason why the version number of Sage is used when creating pickles using sage.structure.sage_object.picklejar
def picklejar(obj, dir=None): """ Create pickled sobj of obj in dir, with name the absolute value of the hash of the pickle of obj. This is used in conjunction with sage.structure.sage_object.unpickle_all. To use this to test the whole Sage library right now, set the environment variable SAGE_PICKLE_JAR, which will make it so dumps will by default call picklejar with the default dir. Once you do that and doctest Sage, you'll find that the SAGE_ROOT/tmp/ contains a bunch of pickled objects along with corresponding txt descriptions of them. Use the sage.structure.sage_object.unpickle_all to see if they unpickle later. INPUTS: - ``obj`` - a pickleable object - ``dir`` - a string or None; if None defaults to ``SAGE_ROOT/tmp/pickle_jar-version``
See #10768 for a potentially conflicting ticket.
Apply:
- 11069_picklejar.patch (positive_review)
- 11069_restore_permissions.patch (needs_review)
Attachments (2)
Change History (13)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- Description modified (diff)
comment:3 Changed 10 years ago by
- Status changed from needs_review to positive_review
comment:4 Changed 10 years ago by
- Reviewers set to Nicolas M. Thiéry
Changed 10 years ago by
comment:5 Changed 10 years ago by
- Description modified (diff)
- Status changed from positive_review to needs_work
comment:6 Changed 10 years ago by
- Status changed from needs_work to needs_review
Additional patch needs review.
comment:7 follow-up: ↓ 8 Changed 10 years ago by
I don't know if it matters in any way, but is there an easy way to restore the original permissions, whatether they are, rather than forcing to 755?
comment:8 in reply to: ↑ 7 Changed 10 years ago by
Replying to nthiery:
I don't know if it matters in any way, but is there an easy way to restore the original permissions, whatether they are, rather than forcing to 755?
I guess one could use os.stat()
but I don't see a reason to add this complication. The only thing which matters is that the directory can be removed without errors.
comment:9 Changed 10 years ago by
Ok, good enough. Please check the failure reported by the buildbot. If it's just a random independent thing, then you can set a positive review on my behalf.
Cheers,
comment:10 Changed 10 years ago by
- Status changed from needs_review to positive_review
It is a random independent thing:
The following tests failed: sage -t -force_lib devel/sage-11069/sage/schemes/plane_conics/all.py # 0 doctests failed
comment:11 Changed 10 years ago by
- Merged in set to sage-4.7.1.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
I myself don't have a strong feeling one way or the other for including the version. Since the release manager is the main user of this function, I happy to abide if he finds it more practical. Beside this harmless change, the patch adds more tests and fixes a typo. And all test pass. So, positive review.