#10354 closed defect (fixed)
Remove deprecated word objects from the pickle jar
Reported by: | slabbe | Owned by: | slabbe |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | pickling | Keywords: | |
Cc: | abmasse, saliola, nthiery | Merged in: | sage-4.7.alpha3 |
Authors: | Sébastien Labbé | Reviewers: | Franco Saliola |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The sage-words google code was merged into sage-3.2.2 two years ago in December 2008 (#4653). At that time, about a dozen of objects were added in the sage pickle jar using that code.
Then, in January 2009, we realized that all of our work was not that good and we started to change all the hierarchy of classes. Basically, we decided after discussions that it was better to get the mathematical classes independent of the chosen representation. This was achieved by ticket #6519 merged in sage-4.1.1 during summer 2009.
Twist in the code were made to make the dozen of old word objects pickled from version < 4.1.1 still usable. As written by Franco Saliola in #6519 :
this way, an old-style word can be unpickled, and during the unpickling, it gets converted to a new-style word, and the user is given a warning to re-save the word [...] This is a temporary fix: since the WordContent code is not necessary for any other part of Sage, it will be deleted in a few months. In the meantime, if there is anyone with some saved word objects, then unpickling will work.
Those deprecation warnings now exists since more than one year, so it's now time to complete the cleaning. As I wrote on sage-combinat devel mailing list in June 2010 :
I would like to remove those 6 pickled object from the pickle jar and replace them with the new generation of words that got in Sage during summer 2009. This way, it would really make sure that we do not break the pickle of objects that are really used. Also, this will allow us to then clean up the code a lot, because some kind of arabesques are being done to support the old stuff... I know William prefer to keep old objects in the pickle jar, but in this case, I would like to remove them.... Will put time on this when I have time...
Then, William answered in the same conversation :
I do prefer keeping old pickles (that's the point of the pickle jar), but I definitely don't want to get in the way of the sage-combinatorics group, and understand that your code is under very active development. Thanks for the work you're doing on cleanup! William
So let's do the cleanup now!
By the way, the current output of the unpickle testing (taken from sage/structure/sage_object.pyx
) is ugly and will be fixed by this ticket :
sage: std = os.environ['SAGE_DATA'] + '/extcode/pickle_jar/pickle_jar.tar.bz2' sage: print "x"; sage.structure.sage_object.unpickle_all(std) x... Successfully unpickled 586 objects. Failed to unpickle 0 objects.
The goal of this ticket is to remove, update or add objects to the pickle jar so that one can remove the deprecated file sage/combinat/words/utils.py
and sage/combinat/words/content.py
. The following objects are OK and can be kepted as is:
- [OK]
_class__sage_combinat_lyndon_word_LyndonWords_evaluation__.sobj
- [OK]
_class__sage_combinat_lyndon_word_LyndonWords_nk__.sobj
- [OK]
_class__sage_combinat_lyndon_word_StandardBracketedLyndonWords_nk__.sobj
- [OK]
_class__sage_combinat_subword_Subwords_w__.sobj
- [OK]
_class__sage_combinat_subword_Subwords_wk__.sobj
- [OK]
_class__sage_combinat_words_alphabet_OrderedAlphabet_Finite__.sobj
- [OK]
_class__sage_combinat_words_alphabet_OrderedAlphabet_NaturalNumbers__.sobj
- [OK]
_class__sage_combinat_words_alphabet_OrderedAlphabet_PositiveIntegers__.sobj
- [OK]
_class__sage_combinat_words_word_generators_WordGenerator__.sobj
- [OK]
_class__sage_combinat_words_words_Words_n__.sobj
The following 19 objects must be deleted or updated because they are affected by the deletion of sage/combinat/words/utils.py
and sage/combinat/words/word_content.py
:
_class__sage_combinat_words_morphism_WordMorphism__.sobj
_class__sage_combinat_words_shuffle_product_ShuffleProduct_overlapping__.sobj
_class__sage_combinat_words_shuffle_product_ShuffleProduct_overlapping_r__.sobj
_class__sage_combinat_words_shuffle_product_ShuffleProduct_shifted__.sobj
_class__sage_combinat_words_shuffle_product_ShuffleProduct_w1w2__.sobj
_class__sage_combinat_words_suffix_trees_ImplicitSuffixTree__.sobj
_class__sage_combinat_words_suffix_trees_SuffixTrie__.sobj
_class__sage_combinat_words_utils_Factorization__.sobj
_class__sage_combinat_words_word_AbstractWord__.sobj
_class__sage_combinat_words_word_Word_over_Alphabet__.sobj
_class__sage_combinat_words_word_Word_over_OrderedAlphabet__.sobj
_class__sage_combinat_words_word_content_WordContentFromFunction__.sobj
_class__sage_combinat_words_word_content_WordContentFromList__.sobj
_class__sage_combinat_words_word_generators_ChristoffelWord_Lower__.sobj
_class__sage_combinat_words_words_FiniteWords_length_k_over_OrderedAlphabet__.sobj
_class__sage_combinat_words_words_FiniteWords_over_OrderedAlphabet__.sobj
_class__sage_combinat_words_words_InfiniteWords_over_OrderedAlphabet__.sobj
_class__sage_combinat_words_words_Words_over_Alphabet__.sobj
_class__sage_combinat_words_words_Words_over_OrderedAlphabet__.sobj
The pickle jar is the file data/extcode/pickle_jar/pickle_jar.tar.bz2
. As a reference, a ticket where objects were removed/updated from the pickle jar: #8911.
Objects added to pickle jar: http://sage.math.washington.edu/home/slabbe/pickle_jar_new_word_objects.tar.bz2
Apply also trac_10354_pickle_jar_test_fix-sl.patch and trac_10354_remove_longtest-sl.patch
Attachments (2)
Change History (32)
comment:1 Changed 11 years ago by
- Description modified (diff)
comment:2 Changed 11 years ago by
- Description modified (diff)
comment:3 Changed 11 years ago by
- Description modified (diff)
comment:4 Changed 11 years ago by
- Description modified (diff)
comment:5 Changed 11 years ago by
- Description modified (diff)
Here is the command I am using to remove the 19 pickled object from the pickle_jar directory :
mv _class__sage_combinat_words_morphism_WordMorphism__.sobj _class__sage_combinat_words_shuffle_product_ShuffleProduct_overlapping__.sobj _class__sage_combinat_words_shuffle_product_ShuffleProduct_overlapping_r__.sobj _class__sage_combinat_words_shuffle_product_ShuffleProduct_shifted__.sobj _class__sage_combinat_words_shuffle_product_ShuffleProduct_w1w2__.sobj _class__sage_combinat_words_suffix_trees_ImplicitSuffixTree__.sobj _class__sage_combinat_words_suffix_trees_SuffixTrie__.sobj _class__sage_combinat_words_utils_Factorization__.sobj _class__sage_combinat_words_word_AbstractWord__.sobj _class__sage_combinat_words_word_Word_over_Alphabet__.sobj _class__sage_combinat_words_word_Word_over_OrderedAlphabet__.sobj _class__sage_combinat_words_word_content_WordContentFromFunction__.sobj _class__sage_combinat_words_word_content_WordContentFromList__.sobj _class__sage_combinat_words_word_generators_ChristoffelWord_Lower__.sobj _class__sage_combinat_words_words_FiniteWords_length_k_over_OrderedAlphabet__.sobj _class__sage_combinat_words_words_FiniteWords_over_OrderedAlphabet__.sobj _class__sage_combinat_words_words_InfiniteWords_over_OrderedAlphabet__.sobj _class__sage_combinat_words_words_Words_over_Alphabet__.sobj _class__sage_combinat_words_words_Words_over_OrderedAlphabet__.sobj ../removed
mv _class__sage_combinat_words_morphism_WordMorphism__.txt _class__sage_combinat_words_shuffle_product_ShuffleProduct_overlapping__.txt _class__sage_combinat_words_shuffle_product_ShuffleProduct_overlapping_r__.txt _class__sage_combinat_words_shuffle_product_ShuffleProduct_shifted__.txt _class__sage_combinat_words_shuffle_product_ShuffleProduct_w1w2__.txt _class__sage_combinat_words_suffix_trees_ImplicitSuffixTree__.txt _class__sage_combinat_words_suffix_trees_SuffixTrie__.txt _class__sage_combinat_words_utils_Factorization__.txt _class__sage_combinat_words_word_AbstractWord__.txt _class__sage_combinat_words_word_Word_over_Alphabet__.txt _class__sage_combinat_words_word_Word_over_OrderedAlphabet__.txt _class__sage_combinat_words_word_content_WordContentFromFunction__.txt _class__sage_combinat_words_word_content_WordContentFromList__.txt _class__sage_combinat_words_word_generators_ChristoffelWord_Lower__.txt _class__sage_combinat_words_words_FiniteWords_length_k_over_OrderedAlphabet__.txt _class__sage_combinat_words_words_FiniteWords_over_OrderedAlphabet__.txt _class__sage_combinat_words_words_InfiniteWords_over_OrderedAlphabet__.txt _class__sage_combinat_words_words_Words_over_Alphabet__.txt _class__sage_combinat_words_words_Words_over_OrderedAlphabet__.txt ../removed
comment:6 Changed 11 years ago by
Here is the pickle jar I suggest for replacement (removed 19 word object mentionned above and added 19 new word objects) :
http://sage.math.washington.edu/home/slabbe/pickle_jar.tar.bz2
This new pickle jar should replace the one found here : SAGE_ROOT/data/extcode/pickle_jar/pickle_jar.tar.bz2
.
comment:7 Changed 11 years ago by
- Status changed from new to needs_review
comment:8 Changed 11 years ago by
- Status changed from needs_review to positive_review
Short version: positive review for the proposed changes and the proposed patch.
Slightly longer version. I dropped the provided tarball
http://sage.math.washington.edu/home/slabbe/pickle_jar.tar.bz2
into place and tested unpickling; everything works:
sage: sage.structure.sage_object.unpickle_all() Successfully unpickled 586 objects. Failed to unpickle 0 objects. sage:
I also compared new pickle jar directory with the old (using diff -rq
), and no other pickles were effected.
comment:9 Changed 11 years ago by
- Milestone changed from sage-4.6.2 to sage-4.7
- Reviewers set to Franco Saliola
comment:10 Changed 11 years ago by
- Description modified (diff)
comment:11 follow-up: ↓ 12 Changed 11 years ago by
- Status changed from positive_review to needs_work
Instead of providing a whole new pickle jar, could you make a tarball containing only the new pickles? I would prefer that for merging as opposed to replacing the pickle jar as a whole.
comment:12 in reply to: ↑ 11 Changed 11 years ago by
- Status changed from needs_work to needs_review
Replying to jdemeyer:
Instead of providing a whole new pickle jar, could you make a tarball containing only the new pickles? I would prefer that for merging as opposed to replacing the pickle jar as a whole.
Already done. A tarball containing the new 19 pickles is here : comment 4. Should Franco take a look at this tarball before putting the status back to positive review?
Also, see comment 5 for the command I use to remove the 19 deprecated objects from the pickle jar.
comment:13 Changed 11 years ago by
Some notes about things I learned on the pickle jar during this ticket:
- Create a Sage object :
sage: w = Word(range(10)) sage: type(w) <class 'sage.combinat.words.word.FiniteWord_list'>
- Add this object to the current pickle jar:
sage: sage.structure.sage_object.picklejar(w)
- By default, the current pickle jar is in
SAGE_ROOT/tmp/pickle_jar-4.6.1
:
cd sage-4.6.1/tmp/pickle_jar-4.6.1 ls _class__sage_combinat_words_word_FiniteWord_list__.sobj _class__sage_combinat_words_word_FiniteWord_list__.txt
- The command to create a .tar.bz2 from a directory:
tar -jcvf archive_name.tar.bz2 directory_to_compress
comment:14 Changed 11 years ago by
- Status changed from needs_review to positive_review
Changed 11 years ago by
comment:15 Changed 11 years ago by
I just refreshed the patch because of conflicts with #10712 merged in sage-4.6.2.
comment:16 follow-ups: ↓ 17 ↓ 19 Changed 11 years ago by
- Status changed from positive_review to needs_work
In a recent thread on sage-combinat-devel, it was mentioned to revert the long test comment on the unpickleall test. I am fixing this here for practical reasons.
This last patch needs review (Nicolas Thierry proposed himself).
Sébastien
comment:17 in reply to: ↑ 16 Changed 11 years ago by
This last patch needs review (Nicolas Thierry proposed himself).
Thiéry, sorry.
comment:18 Changed 11 years ago by
- Cc nthiery added
- Status changed from needs_work to needs_review
comment:19 in reply to: ↑ 16 ; follow-up: ↓ 21 Changed 11 years ago by
Replying to slabbe:
In a recent thread on sage-combinat-devel, it was mentioned to revert the long test comment on the unpickleall test. I am fixing this here for practical reasons.
This last patch needs review (Nicolas Thiéry proposed himself).
Done. Back to positive review! (assuming the builbot is happy, which I assume it will)
comment:20 Changed 11 years ago by
- Status changed from needs_review to positive_review
comment:21 in reply to: ↑ 19 Changed 11 years ago by
Done. Back to positive review! (assuming the builbot is happy, which I assume it will
It will not be "All tests passed!" because the buildbot is testing the applied patches with sage-4.6.2 which still uses the old pickles. But no other error should be raised.
Sébastien
comment:22 Changed 11 years ago by
- Description modified (diff)
comment:23 Changed 11 years ago by
- Description modified (diff)
comment:24 follow-up: ↓ 25 Changed 11 years ago by
- Status changed from positive_review to needs_work
The new pickles (http://sage.math.washington.edu/home/slabbe/pickle_jar_new_word_objects.tar.bz2) are in a directory called pickle_jar-4.6.1
. Is there a reason for this? If not, please fix this.
comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 11 years ago by
The new pickles (http://sage.math.washington.edu/home/slabbe/pickle_jar_new_word_objects.tar.bz2) are in a directory called
pickle_jar-4.6.1
. Is there a reason for this?
Not really. That is the default directory name when one creates pickles :
sage: import os sage: os.listdir(SAGE_ROOT+'/tmp') ['COPYING.txt', 'install', 'README.txt', 'sage-README-osx.txt', 'VERSION.txt'] sage: from sage.structure.sage_object import picklejar sage: picklejar(matrix([0,1,1])) sage: os.listdir(SAGE_ROOT+'/tmp') ['COPYING.txt', 'install', 'pickle_jar-4.6.2', 'README.txt', 'sage-README-osx.txt', 'VERSION.txt'] sage: os.listdir(SAGE_ROOT+'/tmp/pickle_jar-4.6.2') ['_type__sage_matrix_matrix_integer_dense_Matrix_integer_dense__.sobj', '_type__sage_matrix_matrix_integer_dense_Matrix_integer_dense__.txt']
If not, please fix this.
Of course, I can rename the directory pickle_jar-4.6.1
. What name should I chose ?
I think there are no documentation anywhere for adding/removing objects from the pickle jar. So, if there is anything I should do better, we should write it somewhere so that the next time, it will be done the right way.
Sébastien
comment:26 in reply to: ↑ 25 ; follow-up: ↓ 29 Changed 11 years ago by
- Status changed from needs_work to positive_review
Replying to slabbe:
The new pickles (http://sage.math.washington.edu/home/slabbe/pickle_jar_new_word_objects.tar.bz2) are in a directory called
pickle_jar-4.6.1
. Is there a reason for this?Not really. That is the default directory name when one creates pickles :
Okay, point taken. You can leave the file as it is.
Question is: why does the default directory name for creating pickles contain the version number of Sage? It might be better to simply use "$SAGE_ROOT/tmp/pickle_jar" without version number.
comment:27 Changed 11 years ago by
See #11069 for a follow-up.
comment:28 Changed 11 years ago by
- Merged in set to sage-4.7.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
comment:29 in reply to: ↑ 26 Changed 11 years ago by
Question is: why does the default directory name for creating pickles contain the version number of Sage? It might be better to simply use "$SAGE_ROOT/tmp/pickle_jar" without version number.
Personally, I don't have argument for one or the other. But I know that Nicolas Thiéry proposed in #10768 (Revisit the pickle jar procedure) that :
"For each version of Sage (say 4.6.2), a fresh pickle jar (say pickle_jar-4.6.2.tar.bz2) would be produced by the release manager, and *added* to the Sage distribution."
So if this proposition is followed, maybe version number could be kept as a default. Am I confusing the name of the directory with the name of the archive?
Sébastien
Here is a pickle jar made of the objects I am adding to the pickle jar :
http://sage.math.washington.edu/home/slabbe/pickle_jar_new_word_objects.tar.bz2