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

Better Sage consistency for naming and calling in linear_code #17973

Closed
sagetrac-dlucas mannequin opened this issue Mar 17, 2015 · 16 comments
Closed

Better Sage consistency for naming and calling in linear_code #17973

sagetrac-dlucas mannequin opened this issue Mar 17, 2015 · 16 comments

Comments

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Mar 17, 2015

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.

CC: @johanrosenkilde @defeo @nathanncohen

Component: coding theory

Author: David Lucas

Branch/Commit: cd3c6ee

Reviewer: Nathann Cohen

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

@sagetrac-dlucas sagetrac-dlucas mannequin added this to the sage-6.6 milestone Mar 17, 2015
@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 17, 2015

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 17, 2015

New commits:

80770c5Replaced gen_mat by generator_matrix
274d72eChanged names of linear code parameters
f57ba38Class parameters are now accessed by getter methods
7b7395eReplaced check_mat method by parity_check_matrix

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 17, 2015

Commit: 7b7395e

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 17, 2015

Author: David Lucas

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 17, 2015

comment:4

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 17, 2015

Changed commit from 7b7395e to 8813983

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 17, 2015

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

8813983Replaced call to deprecation by the method deprecation_function_alias

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 17, 2015

comment:6

It's done! I also replaced "check matrix" by "parity check matrix" in parity_check_matrix docstring for consistency with its new name.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 17, 2015

comment:8

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 2015-03-17-17-31-39-70361bd3.
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

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 17, 2015

Reviewer: Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 17, 2015

comment:9

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 17, 2015

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

7df4c04Fixed 2 broken doctests
3059874Fixed 4 broken doctests
cd3c6eeFixed 2 broken doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 17, 2015

Changed commit from 8813983 to cd3c6ee

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 17, 2015

comment:11

Thanks for the advice. I was indeed able to find some more after a make -ptestlong.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 17, 2015

comment:13

OKayyyyyyyyyyy then it's good to go !

Nathann

@vbraun
Copy link
Member

vbraun commented Mar 19, 2015

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

1 participant