Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving lyndon_word.py in sage.combinat.words #19150

Closed
nadialafreniere opened this issue Sep 6, 2015 · 30 comments
Closed

Moving lyndon_word.py in sage.combinat.words #19150

nadialafreniere opened this issue Sep 6, 2015 · 30 comments

Comments

@nadialafreniere
Copy link
Contributor

Someone told me that there was a Lyndon word file in sage.combinat that was outside of sage.combinat.words. We moved it.

CC: @egunawan @sagetrac-mlapointe @sagetrac-sschanck @tscrim @sagetrac-tmonteil

Component: combinatorics

Keywords: Lyndon words, days69, fpsac2019

Author: Nadia Lafrenière

Branch/Commit: cb47f9d

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/19150

@nadialafreniere
Copy link
Contributor Author

Branch: u/nadialafreniere/lyndon

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 6, 2015

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

82cb74fMinor corrections to importations in lyndon_word.py

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 6, 2015

Commit: 82cb74f

@nadialafreniere
Copy link
Contributor Author

Changed keywords from Lyndon words to Lyndon words, days69

@jdemeyer
Copy link

jdemeyer commented Sep 7, 2015

comment:4

The old import location should be deprecated: http://doc.sagemath.org/html/en/developer/coding_in_python.html#deprecation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2015

Changed commit from 82cb74f to ef3627d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2015

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

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

@jdemeyer
Copy link

jdemeyer commented Sep 8, 2015

comment:7

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.

@nadialafreniere
Copy link
Contributor Author

Changed branch from u/nadialafreniere/lyndon to none

@nadialafreniere
Copy link
Contributor Author

Changed commit from ef3627d to none

@nadialafreniere
Copy link
Contributor Author

Branch: u/nadialafreniere/moved_lyndon

@nadialafreniere
Copy link
Contributor Author

comment:10

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

@nadialafreniere
Copy link
Contributor Author

Commit: efb4873

@jdemeyer
Copy link

comment:11
  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)

@jdemeyer
Copy link

comment:12

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

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)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

Changed commit from efb4873 to a4c5eff

@nadialafreniere
Copy link
Contributor Author

comment:14

I've modified the patch in accordance with comments 11 and 12.
Thanks for these suggestions.

@jdemeyer
Copy link

comment:15

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`

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2015

Changed commit from a4c5eff to 266ec7f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2015

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

266ec7fChange the reference to Lyndon Words in enumerated_sets.py

@nadialafreniere
Copy link
Contributor Author

comment:17

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.

@jdemeyer
Copy link

comment:18

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.
**********************************************************************

@fchapoton
Copy link
Contributor

Changed commit from 266ec7f to cb47f9d

@fchapoton
Copy link
Contributor

Changed branch from u/nadialafreniere/moved_lyndon to public/ticket/19150

@fchapoton
Copy link
Contributor

comment:21

fresh new branch


New commits:

cb47f9dtrac 19150 moving lyndon words into words

@tscrim
Copy link
Collaborator

tscrim commented Jul 8, 2019

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 8, 2019

comment:22

LGTM.

@tscrim
Copy link
Collaborator

tscrim commented Jul 8, 2019

Changed keywords from Lyndon words, days69 to Lyndon words, days69, fpsac2019

@vbraun
Copy link
Member

vbraun commented Jul 9, 2019

Changed branch from public/ticket/19150 to cb47f9d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants