Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#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:

Status badges

Description (last modified by jdemeyer)

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)

trac_10354_pickle_jar_test_fix-sl.patch (872 bytes) - added by slabbe 11 years ago.
trac_10354_remove_longtest-sl.patch (833 bytes) - added by slabbe 11 years ago.
Applies over the precedent patch

Download all attachments as: .zip

Change History (32)

comment:1 Changed 11 years ago by slabbe

  • Description modified (diff)

comment:2 Changed 11 years ago by slabbe

  • Description modified (diff)

comment:3 Changed 11 years ago by slabbe

  • Description modified (diff)

comment:4 Changed 11 years ago by slabbe

  • Description modified (diff)

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

comment:5 Changed 11 years ago by slabbe

  • 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 slabbe

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 slabbe

  • Authors set to Sébastien Labbé
  • Status changed from new to needs_review

comment:8 Changed 11 years ago by saliola

  • 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 jdemeyer

  • Milestone changed from sage-4.6.2 to sage-4.7
  • Reviewers set to Franco Saliola

comment:10 Changed 11 years ago by jdemeyer

  • Description modified (diff)

comment:11 follow-up: Changed 11 years ago by jdemeyer

  • 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 slabbe

  • 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 slabbe

Some notes about things I learned on the pickle jar during this ticket:

  1. Create a Sage object :
sage: w = Word(range(10))
sage: type(w)
<class 'sage.combinat.words.word.FiniteWord_list'>
  1. Add this object to the current pickle jar:
sage: sage.structure.sage_object.picklejar(w)
  1. 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
  1. 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 jdemeyer

  • Status changed from needs_review to positive_review

Changed 11 years ago by slabbe

comment:15 Changed 11 years ago by slabbe

I just refreshed the patch because of conflicts with #10712 merged in sage-4.6.2.

Changed 11 years ago by slabbe

Applies over the precedent patch

comment:16 follow-ups: Changed 11 years ago by slabbe

  • 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 slabbe

This last patch needs review (Nicolas Thierry proposed himself).

Thiéry, sorry.

comment:18 Changed 11 years ago by slabbe

  • Cc nthiery added
  • Status changed from needs_work to needs_review

comment:19 in reply to: ↑ 16 ; follow-up: Changed 11 years ago by nthiery

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 nthiery

  • Status changed from needs_review to positive_review

comment:21 in reply to: ↑ 19 Changed 11 years ago by slabbe

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 jdemeyer

  • Description modified (diff)

comment:23 Changed 11 years ago by jdemeyer

  • Description modified (diff)

comment:24 follow-up: Changed 11 years ago by jdemeyer

  • 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: Changed 11 years ago by 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 :

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: Changed 11 years ago by jdemeyer

  • 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 jdemeyer

See #11069 for a follow-up.

comment:28 Changed 11 years ago by jdemeyer

  • 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 slabbe

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

comment:30 Changed 11 years ago by jdemeyer

I left some comments at #10768, let's see what happens. In any case, I propose to move the discussion to the relevant tickets: #10768 and #11069 and not anymore on this ticket.

Note: See TracTickets for help on using tickets.