Opened 6 years ago
Closed 6 years ago
#21326 closed enhancement (fixed)
Prevent creation of code with length 0
Reported by:  fdosso48  Owned by:  

Priority:  major  Milestone:  sage7.4 
Component:  coding theory  Keywords:  sd75 
Cc:  dlucas, jlavauzelle, erousseau  Merged in:  
Authors:  Fangan Dosso, Édouard Rousseau  Reviewers:  Bruno Grenet 
Report Upstream:  N/A  Work issues:  
Branch:  c810635 (Commits, GitHub, GitLab)  Commit:  c8106352b7f8fed96dc622d0dee786704579b872 
Dependencies:  Stopgaps: 
Description (last modified by )
AbstractLinearCode
class allows construction of code with length 0.
The goal of this ticket is to prevent this.
Change History (26)
comment:1 Changed 6 years ago by
 Branch set to u/fdosso48/code_with_length_0
comment:2 Changed 6 years ago by
 Commit set to 7b1b3ec5b6042c57a1d1af4d17e8c081b900572f
 Description modified (diff)
comment:3 Changed 6 years ago by
 Commit changed from 7b1b3ec5b6042c57a1d1af4d17e8c081b900572f to c144ddf7445143750434efefcb74bbc472cb23f7
comment:4 Changed 6 years ago by
 Status changed from new to needs_review
comment:5 Changed 6 years ago by
 Branch changed from u/fdosso48/code_with_length_0 to u/erousseau/code_with_length_0
comment:6 Changed 6 years ago by
 Commit changed from c144ddf7445143750434efefcb74bbc472cb23f7 to 25c0758b4b2caa62adf34b7bdf4ed418ae1bd004
Hi,
I just changed the indentation in the file, that didn't fit with Sage conventions. I think it's fine but it needs review.
Best, Édouard
New commits:
6075248  Fixed a bug related to subdivide method

482ae02  added doctest

10460b4  Improves a little the 'subdivide' method documentation

ad3d72a  Merge branch 'u/fdosso48/code_with_length_0' of trac.sagemath.org:sage into code_with_length0

25c0758  I removed some spaces to fit with Sage conventions.

comment:7 Changed 6 years ago by
 Commit changed from 25c0758b4b2caa62adf34b7bdf4ed418ae1bd004 to c144ddf7445143750434efefcb74bbc472cb23f7
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
comment:8 Changed 6 years ago by
Sorry, I did an error, I just fixed my error, not the original fix I did, you can see the diff in 25c0758 just above, just ignore the rest of it...
Best, Édouard
comment:9 Changed 6 years ago by
 Cc erousseau added
comment:10 Changed 6 years ago by
Hello Édouard,
As you just fixed an error related to the indentation level,
I think it's fine if you complete the review process yourself.
It's not a fundamental change, so it's fine :)
David
comment:11 Changed 6 years ago by
Hi Fangan,
It might be good to add a doctest that explains that the length 0 is not available anymore, and to mention this ticket !
Best, Édouard
comment:12 Changed 6 years ago by
Hi,
I agree, it is a good idea.
Best, hoping you are doing it :)
comment:13 Changed 6 years ago by
 Commit changed from c144ddf7445143750434efefcb74bbc472cb23f7 to 8ddca4a5be34beb70ac7325b8b3a179baa38d37f
Branch pushed to git repo; I updated commit sha1. New commits:
8ddca4a  I added a doctest testing the new exception.

comment:14 Changed 6 years ago by
I added a doctest, just as I suggested above. It's open for review.
Best, Édouard
comment:15 Changed 6 years ago by
 Commit changed from 8ddca4a5be34beb70ac7325b8b3a179baa38d37f to d03f2de24bc742962053c02f5abfea81777894b1
Branch pushed to git repo; I updated commit sha1. New commits:
d03f2de  Fixed a typo.

comment:16 followup: ↓ 17 Changed 6 years ago by
 Status changed from needs_review to needs_work
Two doctests are broken in src/sage/coding/encoder.py
(lines 204 & 207). Actually, these doctests are specifically for the behavior of encoders with length equal to 0!
comment:17 in reply to: ↑ 16 ; followup: ↓ 18 Changed 6 years ago by
Replying to bruno:
Two doctests are broken in
src/sage/coding/encoder.py
(lines 204 & 207). Actually, these doctests are specifically for the behavior of encoders with length equal to 0!
You are totally right, should I just remove those doctests then ?
comment:18 in reply to: ↑ 17 ; followup: ↓ 19 Changed 6 years ago by
Replying to erousseau:
Replying to bruno:
Two doctests are broken in
src/sage/coding/encoder.py
(lines 204 & 207). Actually, these doctests are specifically for the behavior of encoders with length equal to 0!You are totally right, should I just remove those doctests then ?
That may be the right thing to do yes. You may replace the whole block of doctests by something like
Note that since ticket :trac:`21326`, codes cannot be of length zero:: sage: G = Matrix(GF(17),[]) sage: C = LinearCode(G) Traceback (most recent call last): ... ValueError: length must be a nonzero positive integer
My only hesitation is whether the presence of these doctests in src/sage/coding/encoder.py
are a indication that codes of length zero may be useful to have. I really have no idea what this should be, but you may asked David or Johan just to be sure.
comment:19 in reply to: ↑ 18 ; followup: ↓ 20 Changed 6 years ago by
My only hesitation is whether the presence of these doctests in
src/sage/coding/encoder.py
are a indication that codes of length zero may be useful to have. I really have no idea what this should be, but you may asked David or Johan just to be sure.
Actually, I asked David and Johan before doing this method. So, it's ok, the block of doctests can be replaced by what you propose.
comment:20 in reply to: ↑ 19 Changed 6 years ago by
Replying to fdosso48:
My only hesitation is whether the presence of these doctests in
src/sage/coding/encoder.py
are a indication that codes of length zero may be useful to have. I really have no idea what this should be, but you may asked David or Johan just to be sure.Actually, I asked David and Johan before doing this method. So, it's ok, the block of doctests can be replaced by what you propose.
OK, I let you doing so. I guess I'll be able to positively review the ticket once this is done.
comment:21 Changed 6 years ago by
 Commit changed from d03f2de24bc742962053c02f5abfea81777894b1 to dfc829ae5fb9443b3821cd9e9cb6f7f4f47fe106
Branch pushed to git repo; I updated commit sha1. New commits:
dfc829a  I changed a doctest that was broken.

comment:22 Changed 6 years ago by
I changed the doctest by the one suggested by Bruno.
Édouard
comment:23 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:24 Changed 6 years ago by
 Branch changed from u/erousseau/code_with_length_0 to u/bruno/code_with_length_0
comment:25 Changed 6 years ago by
 Commit changed from dfc829ae5fb9443b3821cd9e9cb6f7f4f47fe106 to c8106352b7f8fed96dc622d0dee786704579b872
 Reviewers set to Bruno Grenet
 Status changed from needs_review to positive_review
Everything's OK, documentation successfully builds. I add a commit to remove some trailing spaces in the two modified files.
New commits:
c810635  21326: Remove trailaing spaces [reviewer]

comment:26 Changed 6 years ago by
 Branch changed from u/bruno/code_with_length_0 to c8106352b7f8fed96dc622d0dee786704579b872
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Adds an exception that prevent creation of code with length 0