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

Priority:  major  Milestone:  sage6.6 
Component:  coding theory  Keywords:  
Cc:  jsrn, defeo, ncohen  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 7 years ago by
 Cc ncohen added
comment:2 Changed 7 years ago by
 Branch set to u/dlucas/better_sage_consistency_for_naming_and_calling_in_linear_code
comment:3 Changed 7 years ago by
 Commit set to 7b7395eae5ca1897fa9edf445c5317b1ada00584
 Status changed from new to needs_review
comment:4 Changed 7 years ago by
 Status changed from needs_review to 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 7 years ago by
 Commit changed from 7b7395eae5ca1897fa9edf445c5317b1ada00584 to 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 7 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 7 years ago by
 Status changed from needs_work to needs_review
comment:8 Changed 7 years ago by
 Reviewers set to Nathann Cohen
 Status changed from needs_review to 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 7 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 7 years ago by
 Commit changed from 8813983b5f85ceeb0be6377d0248cc1eb19c1e6a to cd3c6eea6fc2f7b4c6412728a5c8ae6a73aa14bd
comment:11 Changed 7 years ago by
Thanks for the advice. I was indeed able to find some more after a make ptestlong
.
comment:12 Changed 7 years ago by
 Status changed from needs_work to needs_review
comment:13 Changed 7 years ago by
 Status changed from needs_review to positive_review
OKayyyyyyyyyyy then it's good to go !
Nathann
comment:14 Changed 7 years ago by
 Branch changed from u/dlucas/better_sage_consistency_for_naming_and_calling_in_linear_code to cd3c6eea6fc2f7b4c6412728a5c8ae6a73aa14bd
 Resolution set to fixed
 Status changed from positive_review to 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