Opened 5 years ago

Closed 5 years ago

#19240 closed defect (fixed)

Rename matrix_mod2e_dense to matrix_gf2e_dense

Reported by: jdemeyer Owned by:
Priority: trivial Milestone: sage-6.9
Component: linear algebra Keywords:
Cc: SimonKing Merged in:
Authors: Jeroen Demeyer Reviewers: Simon King
Report Upstream: N/A Work issues:
Branch: 1e47929 (Commits) Commit: 1e47929f96da76765a17545e356a96d2f153e26a
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

The name matrix_mod2e_dense suggests that it's about matrices in ZZ/(2^e)ZZ, while it's really for matrices in GF(2^e).

Since the module and class names are non-public implementation details, there is no need for deprecation.

Change History (15)

comment:1 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/rename_matrix_mod2e_dense_to_matrix_gf2e_dense

comment:3 Changed 5 years ago by jdemeyer

  • Commit set to 1e6264db7d78ff3304f59a1f7744e5452c0ca946
  • Description modified (diff)

New commits:

1e6264dRename matrix_mod2e_dense to matrix_gf2e_dense

comment:4 Changed 5 years ago by SimonKing

Even if you think that no deprecation is needed, I do believe that it is at least needed to be able to read old pickles of matrices over GF(2^e). Users are likely to have those stored somewhere, as part of ubiquitous applications.

I don't see how you take care of that in your commit.

comment:5 Changed 5 years ago by git

  • Commit changed from 1e6264db7d78ff3304f59a1f7744e5452c0ca946 to fec4bf142fa9e90cbbe9cbd9f7869fa0a1e43b12

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

fec4bf1Fix unpickling old Matrix_mod2e_dense instances

comment:6 Changed 5 years ago by jdemeyer

  • Status changed from new to needs_review

comment:7 Changed 5 years ago by git

  • Commit changed from fec4bf142fa9e90cbbe9cbd9f7869fa0a1e43b12 to 26f359e6e8011f7547b044a680de3af992a2dfda

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

26f359eFix unpickling old Matrix_mod2e_dense instances

comment:8 Changed 5 years ago by SimonKing

  • Reviewers set to Simon King
  • Status changed from needs_review to needs_work

Apparently there is a malformed docstring:

Error building the documentation.
Traceback (most recent call last):
  File "/home/king/Sage/git/sage/src/doc/common/builder.py", line 1626, in <module>
    getattr(get_builder(name), type)()
  File "/home/king/Sage/git/sage/src/doc/common/builder.py", line 292, in _wrapper
    getattr(get_builder(document), 'inventory')(*args, **kwds)
  File "/home/king/Sage/git/sage/src/doc/common/builder.py", line 503, in _wrapper
    x.get(99999)
  File "/home/king/Sage/git/sage/local/lib/python/multiprocessing/pool.py", line 558, in get
    raise self._value
OSError: [matrices ] docstring of sage.matrix.matrix_gf2e_dense.unpickle_matrix_gf2e_dense_v0:15: WARNING: Block quote ends without a blank line; unexpected unindent.

I try to fix it.

comment:9 Changed 5 years ago by SimonKing

  • Branch changed from u/jdemeyer/rename_matrix_mod2e_dense_to_matrix_gf2e_dense to u/SimonKing/rename_matrix_mod2e_dense_to_matrix_gf2e_dense

comment:10 Changed 5 years ago by SimonKing

  • Branch changed from u/SimonKing/rename_matrix_mod2e_dense_to_matrix_gf2e_dense to u/jdemeyer/rename_matrix_mod2e_dense_to_matrix_gf2e_dense

Done. Turn """ into r""".

comment:11 Changed 5 years ago by SimonKing

  • Branch changed from u/jdemeyer/rename_matrix_mod2e_dense_to_matrix_gf2e_dense to u/SimonKing/rename_matrix_mod2e_dense_to_matrix_gf2e_dense
  • Commit changed from 26f359e6e8011f7547b044a680de3af992a2dfda to 32b2cb18fb11502661084810914ac9ba6e4a1336
  • Status changed from needs_work to needs_review

Why is trac automatically changing the branch back to the old value?? Odd.

comment:12 Changed 5 years ago by git

  • Commit changed from 32b2cb18fb11502661084810914ac9ba6e4a1336 to 1e47929f96da76765a17545e356a96d2f153e26a

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

1e47929Change old into new style raise statement

comment:13 Changed 5 years ago by SimonKing

You didn't actually *introduce* the old-style raise statements that the plugin is complaining about, but anyway I'm fixing them...

comment:14 Changed 5 years ago by SimonKing

  • Status changed from needs_review to positive_review

Since tests pass and the code and docstrings now seem fine, I'm giving it a positive review.

Unless of course you don't like my reviewer commits, or unless you worry about the patchbot reporting a slowdown in startup time (which I find hard to believe, given that you merely rename a module).

comment:15 Changed 5 years ago by vbraun

  • Branch changed from u/SimonKing/rename_matrix_mod2e_dense_to_matrix_gf2e_dense to 1e47929f96da76765a17545e356a96d2f153e26a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.