Opened 8 years ago
Closed 8 years ago
#17973 closed enhancement (fixed)
Better Sage consistency for naming and calling in linear_code
Reported by:  David Lucas  Owned by:  

Priority:  major  Milestone:  sage6.6 
Component:  coding theory  Keywords:  
Cc:  Johan Rosenkilde, Luca De Feo, Nathann Cohen  Merged in:  
Authors:  David Lucas  Reviewers:  Nathann Cohen 
Report Upstream:  N/A  Work issues:  
Branch:  cd3c6ee (Commits, GitHub, GitLab)  Commit:  cd3c6eea6fc2f7b4c6412728a5c8ae6a73aa14bd 
Dependencies:  Stopgaps: 
Description
Some method and parameter names in linear_code.py file are abbreviated names which might be confusing (for instance the distance
parameter actually stands for the minimum distance).
Most importantly, the gen_mat
method will be renamed generator_matrix
and the check_mat
method parity_check_matrix
.
Besides, some getter methods to access the private fields of linear codes exist but are not used internally in the class. To support subclassing, it is better to use these instead of directly invoking a parameter.
Change History (14)
comment:1 Changed 8 years ago by
Cc:  Nathann Cohen added 

comment:2 Changed 8 years ago by
Branch:  → u/dlucas/better_sage_consistency_for_naming_and_calling_in_linear_code 

comment:3 Changed 8 years ago by
Authors:  → David Lucas 

Commit:  → 7b7395eae5ca1897fa9edf445c5317b1ada00584 
Status:  new → needs_review 
comment:4 Changed 8 years ago by
Status:  needs_review → needs_work 

Hello !
I would say that this is good to go, except for one detail: could you deprecate the methods using deprecated_function_alias
instead of doing it manually? There are some advantages, like a warning in the doc of the deprecated function. Besides, they do not appear in the lists of undocumented functions.
http://www.sagemath.org/doc/developer/coding_in_python.html#deprecation
Nathann
comment:5 Changed 8 years ago by
Commit:  7b7395eae5ca1897fa9edf445c5317b1ada00584 → 8813983b5f85ceeb0be6377d0248cc1eb19c1e6a 

Branch pushed to git repo; I updated commit sha1. New commits:
8813983  Replaced call to deprecation by the method deprecation_function_alias

comment:6 Changed 8 years ago by
It's done! I also replaced "check matrix" by "parity check matrix" in parity_check_matrix
docstring for consistency with its new name.
comment:7 Changed 8 years ago by
Status:  needs_work → needs_review 

comment:8 Changed 8 years ago by
Reviewers:  → Nathann Cohen 

Status:  needs_review → needs_work 
There are two broken doctests in binary_code.pyx:
(tmp✔)~/sage/coding$ sage tp 4 l binary_code.pyx too many failed tests, not using stored timings Running doctests with ID 2015031717313970361bd3. Git branch: tmp Doctesting 1 file using 4 threads. sage t long binary_code.pyx ********************************************************************** File "binary_code.pyx", line 1100, in sage.coding.binary_code.BinaryCode.apply_permutation Failed example: B = BinaryCode(codes.ExtendedBinaryGolayCode().gen_mat()) Expected nothing Got: doctest:1: DeprecationWarning: gen_mat is deprecated. Please use generator_matrix instead. See http://trac.sagemath.org/17973 for details. ********************************************************************** File "binary_code.pyx", line 3858, in sage.coding.binary_code.BinaryCodeClassifier.put_in_canonical_form Failed example: B = BinaryCode(codes.ExtendedBinaryGolayCode().gen_mat()) Expected nothing Got: doctest:1: DeprecationWarning: gen_mat is deprecated. Please use generator_matrix instead. See http://trac.sagemath.org/17973 for details. ********************************************************************** 2 items had failures: 1 of 6 in sage.coding.binary_code.BinaryCode.apply_permutation 1 of 8 in sage.coding.binary_code.BinaryCodeClassifier.put_in_canonical_form [351 tests, 2 failures, 6.60 s]  sage t long binary_code.pyx # 2 doctests failed  Total time for all tests: 6.7 seconds cpu time: 6.3 seconds cumulative wall time: 6.6 seconds
comment:9 Changed 8 years ago by
And many others in src/doc/en/constructions/linear_codes.rst
. In some situations it is safer to run all of Sage's doctests just to make sure. There may be others in places I could not guess.
Nathann
comment:10 Changed 8 years ago by
Commit:  8813983b5f85ceeb0be6377d0248cc1eb19c1e6a → cd3c6eea6fc2f7b4c6412728a5c8ae6a73aa14bd 

comment:11 Changed 8 years ago by
Thanks for the advice. I was indeed able to find some more after a make ptestlong
.
comment:12 Changed 8 years ago by
Status:  needs_work → needs_review 

comment:13 Changed 8 years ago by
Status:  needs_review → positive_review 

OKayyyyyyyyyyy then it's good to go !
Nathann
comment:14 Changed 8 years ago by
Branch:  u/dlucas/better_sage_consistency_for_naming_and_calling_in_linear_code → cd3c6eea6fc2f7b4c6412728a5c8ae6a73aa14bd 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Replaced gen_mat by generator_matrix
Changed names of linear code parameters
Class parameters are now accessed by getter methods
Replaced check_mat method by parity_check_matrix