Opened 6 years ago
Closed 22 months ago
#19150 closed defect (fixed)
Moving lyndon_word.py in sage.combinat.words
Reported by: | nadialafreniere | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-8.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 follow-up: ↓ 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 sage-combinat.
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 4 years ago by
- Cc tscrim added
comment:20 Changed 4 years ago by
- Cc tmonteil added
comment:21 Changed 22 months 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 22 months ago by
- Milestone changed from sage-6.9 to sage-8.9
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
LGTM.
comment:23 Changed 22 months ago by
- Keywords fpsac2019 added
comment:24 Changed 22 months 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