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: | sage-7.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 follow-up: ↓ 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 ; follow-up: ↓ 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 ; follow-up: ↓ 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 non-zero 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 ; follow-up: ↓ 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