Clean up the code and the documentation for the coding section

[klee]

Lots of cleaning up of sage/coding folder:

• reorganize the section on coding in the documentation

• PEP8 fixes

• 100% coverage for doctests

• pyflakes fixes

• rename bch.py to bch_code.py and grs.py to grs_code.py

• cleanups for goppa.py

• adding random_received_vector method to linear codes

and other small fixes.

[git]

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

​e0e2dd8

Lots of code polishing

[git]

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

​8ca732d

Removed a deprecation notice

[git]

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

​f06f23f

Fix a typo

[git]

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

​9707a5d

Merge branch 'develop'

[git]

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

​1458655	Merge branch 'develop' into codes-trac27634
​4dbc2a8	Merge branch 'develop' into codes-trac27634

[git]

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

​dbb26e8

Clean up coding section

[git]

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

​ceb7dac

Merge branch 'develop' into codes-1-trac27634

[git]

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

​a1eec5d	Merge branch 'develop'
​8706828	Merge branch 'develop'

[git]

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

​4fd1b78	Merge fixes
​1223c53	Merge branch 'develop'

[embray]

Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).

[git]

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

​8f81da2	Merge branch 'develop'
​171365f	pyflakes fixes on goppa.py
​835b5b5	Docstring fixes for goppa.py

[git]

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

​df55fbd

More docstring fixes for goppa.py

[git]

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

​891e189

Add AUTHORS section to goppa.py

[gh-927589452]

removed was fixed in the latest of the commits

[git]

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

​902b750	Merge branch 'develop'
​c8706ee	Merge branch 'develop'

[jsrn]

This is very nice and thorough cleanup. Here are some comments from glancing through the code (I'm currently compiling):

• You've added forwarding of arguments to Decoder.connected_encoder(). But this is semantically not what was intended with the method. For decode_to_message, the decoder must have a specific type of encoder in mind (which is, in many cases, the same as having a specific generator matrix in mind), and this method enables the user to get exactly that encoder. So the user should not be required, indeed allowed, to supply any arguments to that encoder. How would he know which ones to give? If a sub-class of Decoder uses an encoder with specific arguments forwarded to it, then that decoder class must override connected_encoder and return the correct encoder. The _connected_encoder_name is simply a shorthand which covers most cases.

In short, I recommend you roll back this particular change.

• Since we're already modifying _repr_ for Goppa code, let's add their field, i.e. change their representation to e.g. [8, 2] Goppa code over GF(2).

• The documentation for __eq__ is unhelpful. Let's change it into something like

Test whether ``other`` defines the same Goppa code in the sense that defining
set and generating polynomial is the same.

And then under EXAMPLES document this surprising behaviour of equality:

Note that this check will be false if e.g. ``other`` represents the same
linear code as ``self`` but not constructed as a Goppa code:

sage: E = LinearCode(C.generator_matrix())
sage C == E
False

A similar caveat should be placed on several other code classes, actually. If you feel motivated to put those in, I'd be happy, but I'm not insisting ;-)

• I disagree with your abbreviation in the first line of the module doc of grs_code.py. Writing out "Generalized Reed-Solomon codes" in full is more helpful.

• The first line of the module doc of hamming_code.py should say "Hamming codes", plural.

• Module doc of linear_code.py second paragraph

   (...) `n-k`-dimensional (...)  ->>  `(n-k)`-dimensional

• In linear_code.py, I disagree with your abbreviation of the error message when using not registered encoders/decoders (around line 506). Your shorter error messages are much less helpful.

• I'm not sure I like random_received_vector and its friend. It is a very specific use case to add exactly t errors to a codeword, and for this I don't mind the user typing out a few extra lines by using the StaticErrorRateChannel, which also introduces him/her to the much broader concept of channels. Rather, I'm in favour of adding to the class/module documentation how to use the channels (though it is also in the coding theory tutorial)

If you insist on those methods, random_received_vector_and_codeword could (should) be compactly implemented by using the StaticErrorRateChannel channel.

The documentation of the random_received_vector functions should perhaps be clarified by adding something like the following paragraph:

Note that even though the returned vector is assured to be distance exactly
``distance`` from some codeword in ``self``, it might still be closer than
``distance`` to another codeword if ``distance`` is more than `d/2`, where `d`
is the minimum distance of ``self``.

Perhaps random_received_vector_and_codeword should be obtained by an optional argument to random_received_vector called `return_codeword = False. Note that then the doc should have an OUTPUT` block to describe the output semantics.

The doc states that distance should be positive, but it should say "non-negative".

Best, Johan

[jsrn]

• I'm now browsing through the compiled doc of linear_code.py, and "self" (with only one back-tick) appears quite often. Please do a search-replace for this.

• I disagree with your renaming "Derived code constructions" to "Code constructions" in the main doc for the coding module. The latter is very confusable with e.g. "the quadratic residue code construction", amplified by the file code_constructions.py containing exactly such constructions. I suggest using Derived codes or keeping Derived code constructions.

• I discovered that the documentation for codes_catalog is completely broken (see under the main documentation of Coding, under "Index of code constructions"). The reason is that the introspective magic function that should generate the table doesn't really work with imported non-local classes. I've manually written the entire table in full and pushed that to the branch.

---

New commits:

​0b95c1d

Manual table for codes_catalog.py

[klee]

• You've added forwarding of arguments to Decoder.connected_encoder(). But this is semantically not what was intended with the method. For decode_to_message, the decoder must have a specific type of encoder in mind (which is, in many cases, the same as having a specific generator matrix in mind), and this method enables the user to get exactly that encoder. So the user should not be required, indeed allowed, to supply any arguments to that encoder. How would he know which ones to give? If a sub-class of Decoder uses an encoder with specific arguments forwarded to it, then that decoder class must override connected_encoder and return the correct encoder. The _connected_encoder_name is simply a shorthand which covers most cases.

In short, I recommend you roll back this particular change.

I agree. Done rolling back.

• Since we're already modifying _repr_ for Goppa code, let's add their field, i.e. change their representation to e.g. [8, 2] Goppa code over GF(2).

Done.

• The documentation for __eq__ is unhelpful. Let's change it into something like

Test whether ``other`` defines the same Goppa code in the sense that defining
set and generating polynomial is the same.

I added your text after the original.

And then under EXAMPLES document this surprising behaviour of equality:

Note that this check will be false if e.g. ``other`` represents the same
linear code as ``self`` but not constructed as a Goppa code:

sage: E = LinearCode(C.generator_matrix())
sage C == E
False

Done.

A similar caveat should be placed on several other code classes, actually. If you feel motivated to put those in, I'd be happy, but I'm not insisting ;-)

This behavior of equality check is common in Sage. I don't worry much. But I will add more examples if I feel motivated :-)

• I disagree with your abbreviation in the first line of the module doc of grs_code.py. Writing out "Generalized Reed-Solomon codes" in full is more helpful.

Right. Done.

• The first line of the module doc of hamming_code.py should say "Hamming codes", plural.

Done.

• Module doc of linear_code.py second paragraph

   (...) `n-k`-dimensional (...)  ->>  `(n-k)`-dimensional

• In linear_code.py, I disagree with your abbreviation of the error message when using not registered encoders/decoders (around line 506). Your shorter error messages are much less helpful.

I felt that it was too much verbose. The user should learn what the message says from the documentation.

But ok as I respect your wisdom. I recovered the original message.

• I'm not sure I like random_received_vector and its friend. It is a very specific use case to add exactly t errors to a codeword, and for this I don't mind the user typing out a few extra lines by using the StaticErrorRateChannel, which also introduces him/her to the much broader concept of channels. Rather, I'm in favour of adding to the class/module documentation how to use the channels (though it is also in the coding theory tutorial)

I added that when I was moving my code in Magma to Sage, without much appreciation of the channel concept myself. Now I see that random_received_vector does not fit well with the overall design of the coding module.

I decided to remove it.

[klee]

Replying to jsrn:

• I'm now browsing through the compiled doc of linear_code.py, and "self" (with only one back-tick) appears quite often. Please do a search-replace for this.

Done.

• I disagree with your renaming "Derived code constructions" to "Code constructions" in the main doc for the coding module. The latter is very confusable with e.g. "the quadratic residue code construction", amplified by the file code_constructions.py containing exactly such constructions. I suggest using Derived codes or keeping Derived code constructions.

It seems a typo... I will keep Derived code constructions.

• I discovered that the documentation for codes_catalog is completely broken (see under the main documentation of Coding, under "Index of code constructions"). The reason is that the introspective magic function that should generate the table doesn't really work with imported non-local classes. I've manually written the entire table in full and pushed that to the branch.

Merged. Fixed a doctest failure along with it.

[klee]

New commits:

​fb35c2f	Fixes responding to reviewer comments
​1bca13c	Merge branch 'u/jsrn/27634'
​6bf9f1e	Remove unwanted names under codes
​24df329	A minor fix in the main document

[git]

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

​6c7012b

A pyflakes fix in linear_code.py

[jsrn]

Great, thanks for reacting so fast! Let's get this into Sage :-) Positive review.

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​c650b8a

Rename channel_constructions.py and goppa.py

[klee]

Ah... I could not resist rushing these final changes to this ticket. Sorry.

I renamed "channel_constructions.py" to "channel.py" and "goppa.py" to "goppa_code.py". This seems to me more conforming to other file names.

I worry that you may object to the former change. If so, let me know.

[jsrn]

I'm OK with those changes, it's more conforming. Positive review.

[vbraun]

See patchbot: {{{sage -t --long src/doc/en/thematic_tutorials/structures_in_coding_theory.rst # 2 doc tests failed }}}

[git]

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

​8b01cc5

Rename coding theory module names in thematic tutorials

[klee]

Doctest failures in thematic tutorials were fixed by simple string replacements for changed module names.