Opened 6 years ago
Closed 2 years ago
#19150 closed defect (fixed)
Moving lyndon_word.py in sage.combinat.words
Reported by:  nadialafreniere  Owned by:  

Priority:  minor  Milestone:  sage8.9 
Component:  combinatorics  Keywords:  Lyndon words, days69, fpsac2019 
Cc:  egunawan, mlapointe, sschanck, tscrim, tmonteil  Merged in:  
Authors:  Nadia Lafrenière  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  cb47f9d (Commits, GitHub, GitLab)  Commit:  cb47f9d5e1f54811abf6e3af5d5cc14a7f828df2 
Dependencies:  Stopgaps: 
Description
Someone told me that there was a Lyndon word file in sage.combinat that was outside of sage.combinat.words. We moved it.
Change History (24)
comment:1 Changed 6 years ago by
 Branch set to u/nadialafreniere/lyndon
comment:2 Changed 6 years ago by
 Commit set to 82cb74f82cb1987cb5b584d545955c3e358de79e
comment:3 Changed 6 years ago by
 Keywords days69 added
 Status changed from new to needs_review
comment:4 Changed 6 years ago by
The old import location should be deprecated: http://doc.sagemath.org/html/en/developer/coding_in_python.html#deprecation
comment:5 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:6 Changed 6 years ago by
 Commit changed from 82cb74f82cb1987cb5b584d545955c3e358de79e to ef3627d2541435b4d82e2c6990ee58063aa16deb
comment:7 Changed 6 years ago by
I think you misunderstand how deprecation works. There should be a deprecation in the file src/sage/combinat/lyndon_word.py
. Since you moved that, just create a new file with this name containing only the deprecation and a doctest. The doctest should show that
sage: from sage.combinat.lyndon_word import LyndonWord
still works.
comment:8 Changed 6 years ago by
 Branch u/nadialafreniere/lyndon deleted
 Commit ef3627d2541435b4d82e2c6990ee58063aa16deb deleted
comment:9 Changed 6 years ago by
 Branch set to u/nadialafreniere/moved_lyndon
comment:10 Changed 6 years ago by
 Commit set to efb48730e10a85726211954093f594db275935c8
 Status changed from needs_work to needs_review
I think I've done the deprecation in a proper way, though I'm not so sure... The developer's guide is not so clear about it for someone who never did it.
I started a new branch since I changed the version of sage I had on my computer in between. It's now based on 6.9beta5.
New commits:
3d3d135  Modified location of Lyndon word, added deprecation

b2ee0a8  trying to fix deprecation

efb4873  deprecation fixed

comment:11 Changed 6 years ago by
 Status changed from needs_review to needs_work
 You cannot just put docstrings in arbitrary places. So in
src/sage/combinat/lyndon_word.py
, you should replace""" docstring """ code """ doctests """
by
""" docstring doctests """ code
 The indentation of the docstring is wrong:
TESTS:: sage: ...
should become
TESTS:: sage: ...
 Instead of testing
import *
, I would prefer to see a more concrete import (sayLyndonWord
)
comment:12 Changed 6 years ago by
One more thing: you should also edit src/doc/en/reference/combinat/module_list.rst
comment:13 Changed 6 years ago by
 Commit changed from efb48730e10a85726211954093f594db275935c8 to a4c5eff09cbf130a393dd69f0d35516417e3ab52
Branch pushed to git repo; I updated commit sha1. New commits:
a4c5eff  Moving doctests in lyndon_word.py + editing the list of modules for documentation (reviewer's suggestions)

comment:14 Changed 6 years ago by
 Status changed from needs_work to needs_review
I've modified the patch in accordance with comments 11 and 12. Thanks for these suggestions.
comment:15 followup: ↓ 17 Changed 6 years ago by
 Status changed from needs_review to needs_work
The file src/sage/combinat/enumerated_sets.py
still has a reference to the old file:
Words   :class:`Words`  :ref:`sage.combinat.subword`  :ref:`sage.combinat.necklace`  :ref:`sage.combinat.lyndon_word`  :ref:`sage.combinat.dyck_word`  :ref:`sage.combinat.debruijn_sequence`  :ref:`sage.combinat.shuffle`
comment:16 Changed 6 years ago by
 Commit changed from a4c5eff09cbf130a393dd69f0d35516417e3ab52 to 266ec7fb89615c668e9bf4c139374f5b66f2aa03
Branch pushed to git repo; I updated commit sha1. New commits:
266ec7f  Change the reference to Lyndon Words in enumerated_sets.py

comment:17 in reply to: ↑ 15 Changed 6 years ago by
 Status changed from needs_work to needs_review
Replying to jdemeyer:
The file
src/sage/combinat/enumerated_sets.py
still has a reference to the old file:Words   :class:`Words`  :ref:`sage.combinat.subword`  :ref:`sage.combinat.necklace`  :ref:`sage.combinat.lyndon_word`  :ref:`sage.combinat.dyck_word`  :ref:`sage.combinat.debruijn_sequence`  :ref:`sage.combinat.shuffle`
I changed it. It doesn't seem to appear anywhere else, at least not in sagecombinat.
comment:18 Changed 6 years ago by
 Status changed from needs_review to needs_work
There is a doctest failure (don't ask me exactly why):
sage t long src/sage/structure/sage_object.pyx ********************************************************************** File "src/sage/structure/sage_object.pyx", line 1451, in sage.structure.sage_object.unpickle_all Failed example: sage.structure.sage_object.unpickle_all() # (4s on sage.math, 2011) Expected: doctest:... DeprecationWarning: ... See http://trac.sagemath.org/... for details. Successfully unpickled ... objects. Failed to unpickle 0 objects. Got: doctest:858: DeprecationWarning: You unpickled an object which relies on an old data structure. Save it again to update it, for it may break in the future. See http://trac.sagemath.org/18375 for details. doctest:1: DeprecationWarning: You unpickled an object which relies on an old data structure. Save it again to update it, for it may break in the future. See http://trac.sagemath.org/1000 for details. * unpickle failure: load('/home/jdemeyer/.sage/temp/tamiyo/21030/dir_pG8nxg//pickle_jar/_class__sage_combinat_lyndon_word_LyndonWords_evaluation__.sobj') Traceback (most recent call last): File "sage/structure/sage_object.pyx", line 1543, in sage.structure.sage_object.unpickle_all (build/cythonized/sage/structure/sage_object.c:15114) object = load(os.path.join(dir,A)) File "sage/structure/sage_object.pyx", line 987, in sage.structure.sage_object.load (build/cythonized/sage/structure/sage_object.c:11494) X = loads(open(filename).read(), compress=compress) File "sage/structure/sage_object.pyx", line 1341, in sage.structure.sage_object.loads (build/cythonized/sage/structure/sage_object.c:13583) return unpickler.load() UnpicklingError: NEWOBJ class argument isn't a type object * unpickle failure: load('/home/jdemeyer/.sage/temp/tamiyo/21030/dir_pG8nxg//pickle_jar/_class__sage_combinat_lyndon_word_LyndonWords_nk__.sobj') Traceback (most recent call last): File "sage/structure/sage_object.pyx", line 1543, in sage.structure.sage_object.unpickle_all (build/cythonized/sage/structure/sage_object.c:15114) object = load(os.path.join(dir,A)) File "sage/structure/sage_object.pyx", line 987, in sage.structure.sage_object.load (build/cythonized/sage/structure/sage_object.c:11494) X = loads(open(filename).read(), compress=compress) File "sage/structure/sage_object.pyx", line 1341, in sage.structure.sage_object.loads (build/cythonized/sage/structure/sage_object.c:13583) return unpickler.load() UnpicklingError: NEWOBJ class argument isn't a type object * unpickle failure: load('/home/jdemeyer/.sage/temp/tamiyo/21030/dir_pG8nxg//pickle_jar/_class__sage_combinat_lyndon_word_StandardBracketedLyndonWords_nk__.sobj') Traceback (most recent call last): File "sage/structure/sage_object.pyx", line 1543, in sage.structure.sage_object.unpickle_all (build/cythonized/sage/structure/sage_object.c:15114) object = load(os.path.join(dir,A)) File "sage/structure/sage_object.pyx", line 987, in sage.structure.sage_object.load (build/cythonized/sage/structure/sage_object.c:11494) X = loads(open(filename).read(), compress=compress) File "sage/structure/sage_object.pyx", line 1341, in sage.structure.sage_object.loads (build/cythonized/sage/structure/sage_object.c:13583) return unpickler.load() UnpicklingError: NEWOBJ class argument isn't a type object doctest:1: DeprecationWarning: This function is only used to unpickle invalid objects See http://trac.sagemath.org/18848 for details. doctest:1: DeprecationWarning: OrderedAlphabet is deprecated; use Alphabet instead. See http://trac.sagemath.org/8920 for details. doctest:1: DeprecationWarning: gen_py.pari is deprecated, use sage.libs.pari.all.pari instead See http://trac.sagemath.org/17451 for details. doctest:632: DeprecationWarning: The "pari_mod" finite field implementation is deprecated See http://trac.sagemath.org/17297 for details. doctest:1: DeprecationWarning: MatrixSpace_ZZ_2x2 is deprecated. Please use MatrixSpace(ZZ,2) instead See http://trac.sagemath.org/17824 for details. Failed: _class__sage_combinat_lyndon_word_LyndonWords_evaluation__.sobj _class__sage_combinat_lyndon_word_LyndonWords_nk__.sobj _class__sage_combinat_lyndon_word_StandardBracketedLyndonWords_nk__.sobj  ** This error is probably due to an old pickle failing to unpickle. ** See sage.structure.sage_object.register_unpickle_override for ** how to override the default unpickling methods for (old) pickles. ** NOTE: pickles should never be removed from the pickle_jar!  Successfully unpickled 583 objects. Failed to unpickle 3 objects. ********************************************************************** File "src/sage/structure/sage_object.pyx", line 1459, in sage.structure.sage_object.unpickle_all Failed example: sage.structure.sage_object.unpickle_all() Expected: Successfully unpickled ... objects. Failed to unpickle 0 objects. Got: * unpickle failure: load('/home/jdemeyer/.sage/temp/tamiyo/21030/dir_PjMOnq//pickle_jar/_class__sage_combinat_lyndon_word_LyndonWords_evaluation__.sobj') Traceback (most recent call last): File "sage/structure/sage_object.pyx", line 1543, in sage.structure.sage_object.unpickle_all (build/cythonized/sage/structure/sage_object.c:15114) object = load(os.path.join(dir,A)) File "sage/structure/sage_object.pyx", line 987, in sage.structure.sage_object.load (build/cythonized/sage/structure/sage_object.c:11494) X = loads(open(filename).read(), compress=compress) File "sage/structure/sage_object.pyx", line 1341, in sage.structure.sage_object.loads (build/cythonized/sage/structure/sage_object.c:13583) return unpickler.load() UnpicklingError: NEWOBJ class argument isn't a type object * unpickle failure: load('/home/jdemeyer/.sage/temp/tamiyo/21030/dir_PjMOnq//pickle_jar/_class__sage_combinat_lyndon_word_LyndonWords_nk__.sobj') Traceback (most recent call last): File "sage/structure/sage_object.pyx", line 1543, in sage.structure.sage_object.unpickle_all (build/cythonized/sage/structure/sage_object.c:15114) object = load(os.path.join(dir,A)) File "sage/structure/sage_object.pyx", line 987, in sage.structure.sage_object.load (build/cythonized/sage/structure/sage_object.c:11494) X = loads(open(filename).read(), compress=compress) File "sage/structure/sage_object.pyx", line 1341, in sage.structure.sage_object.loads (build/cythonized/sage/structure/sage_object.c:13583) return unpickler.load() UnpicklingError: NEWOBJ class argument isn't a type object * unpickle failure: load('/home/jdemeyer/.sage/temp/tamiyo/21030/dir_PjMOnq//pickle_jar/_class__sage_combinat_lyndon_word_StandardBracketedLyndonWords_nk__.sobj') Traceback (most recent call last): File "sage/structure/sage_object.pyx", line 1543, in sage.structure.sage_object.unpickle_all (build/cythonized/sage/structure/sage_object.c:15114) object = load(os.path.join(dir,A)) File "sage/structure/sage_object.pyx", line 987, in sage.structure.sage_object.load (build/cythonized/sage/structure/sage_object.c:11494) X = loads(open(filename).read(), compress=compress) File "sage/structure/sage_object.pyx", line 1341, in sage.structure.sage_object.loads (build/cythonized/sage/structure/sage_object.c:13583) return unpickler.load() UnpicklingError: NEWOBJ class argument isn't a type object Failed: _class__sage_combinat_lyndon_word_LyndonWords_evaluation__.sobj _class__sage_combinat_lyndon_word_LyndonWords_nk__.sobj _class__sage_combinat_lyndon_word_StandardBracketedLyndonWords_nk__.sobj  ** This error is probably due to an old pickle failing to unpickle. ** See sage.structure.sage_object.register_unpickle_override for ** how to override the default unpickling methods for (old) pickles. ** NOTE: pickles should never be removed from the pickle_jar!  Successfully unpickled 583 objects. Failed to unpickle 3 objects. **********************************************************************
comment:19 Changed 5 years ago by
 Cc tscrim added
comment:20 Changed 5 years ago by
 Cc tmonteil added
comment:21 Changed 2 years ago by
 Branch changed from u/nadialafreniere/moved_lyndon to public/ticket/19150
 Commit changed from 266ec7fb89615c668e9bf4c139374f5b66f2aa03 to cb47f9d5e1f54811abf6e3af5d5cc14a7f828df2
 Status changed from needs_work to needs_review
comment:22 Changed 2 years ago by
 Milestone changed from sage6.9 to sage8.9
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:23 Changed 2 years ago by
 Keywords fpsac2019 added
comment:24 Changed 2 years ago by
 Branch changed from public/ticket/19150 to cb47f9d5e1f54811abf6e3af5d5cc14a7f828df2
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Minor corrections to importations in lyndon_word.py