Opened 4 years ago

Closed 10 days 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) 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 4 years ago by nadialafreniere

  • Branch set to u/nadialafreniere/lyndon

comment:2 Changed 4 years ago by git

  • Commit set to 82cb74f82cb1987cb5b584d545955c3e358de79e

Branch pushed to git repo; I updated commit sha1. New commits:

82cb74fMinor corrections to importations in lyndon_word.py

comment:3 Changed 4 years ago by nadialafreniere

  • Keywords days69 added
  • Status changed from new to needs_review

comment:4 Changed 4 years ago by jdemeyer

comment:5 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:6 Changed 4 years ago by git

  • Commit changed from 82cb74f82cb1987cb5b584d545955c3e358de79e to ef3627d2541435b4d82e2c6990ee58063aa16deb

Branch pushed to git repo; I updated commit sha1. New commits:

0edf846Added depeciation of sage/combinat/Lyndon_word.py
ef3627ddeprecation of the old file

comment:7 Changed 4 years ago by jdemeyer

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 4 years ago by nadialafreniere

  • Branch u/nadialafreniere/lyndon deleted
  • Commit ef3627d2541435b4d82e2c6990ee58063aa16deb deleted

comment:9 Changed 4 years ago by nadialafreniere

  • Branch set to u/nadialafreniere/moved_lyndon

comment:10 Changed 4 years ago by nadialafreniere

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

3d3d135Modified location of Lyndon word, added deprecation
b2ee0a8trying to fix deprecation
efb4873deprecation fixed

comment:11 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  1. 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
  1. The indentation of the docstring is wrong:
    TESTS::
    
    sage: ...
    

should become

TESTS::

    sage: ...
  1. Instead of testing import *, I would prefer to see a more concrete import (say LyndonWord)

comment:12 Changed 4 years ago by jdemeyer

One more thing: you should also edit src/doc/en/reference/combinat/module_list.rst

comment:13 Changed 4 years ago by git

  • Commit changed from efb48730e10a85726211954093f594db275935c8 to a4c5eff09cbf130a393dd69f0d35516417e3ab52

Branch pushed to git repo; I updated commit sha1. New commits:

a4c5effMoving doctests in lyndon_word.py + editing the list of modules for documentation (reviewer's suggestions)

comment:14 Changed 4 years ago by nadialafreniere

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

  • 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 4 years ago by git

  • Commit changed from a4c5eff09cbf130a393dd69f0d35516417e3ab52 to 266ec7fb89615c668e9bf4c139374f5b66f2aa03

Branch pushed to git repo; I updated commit sha1. New commits:

266ec7fChange the reference to Lyndon Words in enumerated_sets.py

comment:17 in reply to: ↑ 15 Changed 4 years ago by nadialafreniere

  • 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 4 years ago by jdemeyer

  • 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 3 years ago by egunawan

  • Cc tscrim added

comment:20 Changed 2 years ago by tmonteil

  • Cc tmonteil added

comment:21 Changed 12 days ago by chapoton

  • 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

fresh new branch


New commits:

cb47f9dtrac 19150 moving lyndon words into words

comment:22 Changed 11 days ago by tscrim

  • 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 11 days ago by tscrim

  • Keywords fpsac2019 added

comment:24 Changed 10 days ago by vbraun

  • Branch changed from public/ticket/19150 to cb47f9d5e1f54811abf6e3af5d5cc14a7f828df2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.