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:  sage6.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 )
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 nonpublic implementation details, there is no need for deprecation.
Change History (15)
comment:1 Changed 5 years ago by
 Description modified (diff)
comment:2 Changed 5 years ago by
 Branch set to u/jdemeyer/rename_matrix_mod2e_dense_to_matrix_gf2e_dense
comment:3 Changed 5 years ago by
 Commit set to 1e6264db7d78ff3304f59a1f7744e5452c0ca946
 Description modified (diff)
comment:4 Changed 5 years ago by
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
 Commit changed from 1e6264db7d78ff3304f59a1f7744e5452c0ca946 to fec4bf142fa9e90cbbe9cbd9f7869fa0a1e43b12
Branch pushed to git repo; I updated commit sha1. New commits:
fec4bf1  Fix unpickling old Matrix_mod2e_dense instances

comment:6 Changed 5 years ago by
 Status changed from new to needs_review
comment:7 Changed 5 years ago by
 Commit changed from fec4bf142fa9e90cbbe9cbd9f7869fa0a1e43b12 to 26f359e6e8011f7547b044a680de3af992a2dfda
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
26f359e  Fix unpickling old Matrix_mod2e_dense instances

comment:8 Changed 5 years ago by
 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
 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
 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
 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
 Commit changed from 32b2cb18fb11502661084810914ac9ba6e4a1336 to 1e47929f96da76765a17545e356a96d2f153e26a
Branch pushed to git repo; I updated commit sha1. New commits:
1e47929  Change old into new style raise statement

comment:13 Changed 5 years ago by
You didn't actually *introduce* the oldstyle raise statements that the plugin is complaining about, but anyway I'm fixing them...
comment:14 Changed 5 years ago by
 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
 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
New commits:
Rename matrix_mod2e_dense to matrix_gf2e_dense