Opened 3 years ago

Closed 3 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) Commit: c8106352b7f8fed96dc622d0dee786704579b872
Dependencies: Stopgaps:

Description (last modified by fdosso48)

AbstractLinearCode class allows construction of code with length 0. The goal of this ticket is to prevent this.

Change History (26)

comment:1 Changed 3 years ago by fdosso48

  • Branch set to u/fdosso48/code_with_length_0

comment:2 Changed 3 years ago by fdosso48

  • Commit set to 7b1b3ec5b6042c57a1d1af4d17e8c081b900572f
  • Description modified (diff)

comment:3 Changed 3 years ago by git

  • Commit changed from 7b1b3ec5b6042c57a1d1af4d17e8c081b900572f to c144ddf7445143750434efefcb74bbc472cb23f7

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

c144ddfAdds an exception that prevent creation of code with length 0

comment:4 Changed 3 years ago by fdosso48

  • Status changed from new to needs_review

comment:5 Changed 3 years ago by erousseau

  • Branch changed from u/fdosso48/code_with_length_0 to u/erousseau/code_with_length_0

comment:6 Changed 3 years ago by erousseau

  • 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:

6075248Fixed a bug related to subdivide method
482ae02added doctest
10460b4Improves a little the 'subdivide' method documentation
ad3d72aMerge branch 'u/fdosso48/code_with_length_0' of trac.sagemath.org:sage into code_with_length0
25c0758I removed some spaces to fit with Sage conventions.

comment:7 Changed 3 years ago by git

  • 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 3 years ago by erousseau

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 3 years ago by erousseau

  • Cc erousseau added

comment:10 Changed 3 years ago by dlucas

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 3 years ago by erousseau

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 3 years ago by fdosso48

Hi,

I agree, it is a good idea.

Best, hoping you are doing it :)

comment:13 Changed 3 years ago by git

  • Commit changed from c144ddf7445143750434efefcb74bbc472cb23f7 to 8ddca4a5be34beb70ac7325b8b3a179baa38d37f

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

8ddca4aI added a doctest testing the new exception.

comment:14 Changed 3 years ago by erousseau

I added a doctest, just as I suggested above. It's open for review.

Best, Édouard

comment:15 Changed 3 years ago by git

  • Commit changed from 8ddca4a5be34beb70ac7325b8b3a179baa38d37f to d03f2de24bc742962053c02f5abfea81777894b1

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

d03f2deFixed a typo.

comment:16 follow-up: Changed 3 years ago by bruno

  • 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: Changed 3 years ago by 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 ?

comment:18 in reply to: ↑ 17 ; follow-up: Changed 3 years ago by bruno

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.
Version 0, edited 3 years ago by bruno (next)

comment:19 in reply to: ↑ 18 ; follow-up: Changed 3 years ago by 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.

Last edited 3 years ago by fdosso48 (previous) (diff)

comment:20 in reply to: ↑ 19 Changed 3 years ago by bruno

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 3 years ago by git

  • Commit changed from d03f2de24bc742962053c02f5abfea81777894b1 to dfc829ae5fb9443b3821cd9e9cb6f7f4f47fe106

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

dfc829aI changed a doctest that was broken.

comment:22 Changed 3 years ago by erousseau

I changed the doctest by the one suggested by Bruno.

Édouard

comment:23 Changed 3 years ago by erousseau

  • Status changed from needs_work to needs_review

comment:24 Changed 3 years ago by bruno

  • Branch changed from u/erousseau/code_with_length_0 to u/bruno/code_with_length_0

comment:25 Changed 3 years ago by bruno

  • Authors changed from Fangan Dosso to Fangan Dosso, Édouard Rousseau
  • 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:

c81063521326: Remove trailaing spaces [reviewer]

comment:26 Changed 3 years ago by vbraun

  • Branch changed from u/bruno/code_with_length_0 to c8106352b7f8fed96dc622d0dee786704579b872
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.