Opened 5 years ago

Closed 5 years ago

#20899 closed defect (fixed)

`AbstractLinearCode` should throw sensible error messages on printing

Reported by: jsrn Owned by:
Priority: major Milestone: sage-7.3
Component: coding theory Keywords: linear code, error messages, rd3
Cc: dlucas Merged in:
Authors: David Lucas Reviewers: Bruno Grenet
Report Upstream: N/A Work issues:
Branch: b0bca6f (Commits, GitHub, GitLab) Commit: b0bca6f978580b57129256264b338e6b0adb6cc1
Dependencies: Stopgaps:

Status badges

Description

The doc explicitly says that to create a new class of linear codes, you must override _repr_ and _latex_. However, if the user forgets, a bizarre error seems to be thrown when trying to print an instance of the new code class:

%cpaste
from sage.coding.linear_code import AbstractLinearCode
from sage.coding.encoder import Encoder

class MyCode(AbstractLinearCode):
    _registered_encoders = {}
    _registered_decoders = {}
    def __init__(self):
        super(MyCode, self).__init__(GF(5), 10, "Monkey", "Syndrome")
        self._dimension = 2

class MonkeyEncoder(Encoder):
    def __init__(self, C):    
        super(MonkeyEncoder, self).__init__(C)
    @cached_method
    def generator_matrix(self): 
        return matrix(GF(5), 2, 10, [ [1]*5 + [0]*5, [0]*5 + [1]*5 ])
MyCode._registered_encoders["Monkey"] = MonkeyEncoder
MyCode._registered_decoders["Syndrome"] = codes.decoders.LinearCodeSyndromeDecoder
--
sage: C = MyCode()
sage: C
<BOOM with bizarre error>

If AbstractLinearCode had a _repr_ and a _latex_ method which both threw sensible error messages, this would be much more helpful to the user.

Change History (8)

comment:1 Changed 5 years ago by jsrn

  • Summary changed from `AbstractLinearCode` shouldn't throw sensible error messages on printing to `AbstractLinearCode` should throw sensible error messages on printing

Fixed amusing spelling mistake in the title...

comment:2 Changed 5 years ago by dlucas

  • Branch set to u/dlucas/abstract_lin_code_print_error_messages

comment:3 Changed 5 years ago by dlucas

  • Commit set to 152c4711a4ca9c79878129cb0b5881e1c315d4cb
  • Status changed from new to needs_review

Hello,

I pushed a fix for this ticket.

Two remarks:

  • I used RuntimeError instead of NotImplementedError because using NotImplementedError prints a poorly formatted error message (you don't get any traceback).
  • Because _repr_ to always returns a string, it returns a string message including the hexadecimal address of the objects which raised the error after the usual error traceback... Because this address changes everytime, I had to set a flag #random to the related doctest.

Best,

David


New commits:

4862c83Moved _latex_ method from AbstractLinearCode to LinearCode
152c471Added generic _repr_ and _latex_ which raise informative error messages

comment:4 Changed 5 years ago by bruno

  • Authors set to David Lucas
  • Reviewers set to Bruno Grenet
  • Status changed from needs_review to needs_work

Two changes and a comment/question in _repr_ and _latex_ of AbstractLinearCode:

  • Returns should be Return (in both docstrings)
  • It would be better to use Python3's print for the future: Please replace "Please override _repr_ in the implementation of %s" % self.parent() by "Please override _repr_ in the implementation of {}".format(self.parent()), and similarly in _latex_.
  • Is it possible to get the error
    RuntimeError: Please override _repr_ in the implementation of MyCode
    
    instead of
    RuntimeError: Please override _repr_ in the implementation of <class __main__.MyCode_with_category'>
    

comment:5 Changed 5 years ago by git

  • Commit changed from 152c4711a4ca9c79878129cb0b5881e1c315d4cb to b0bca6f978580b57129256264b338e6b0adb6cc1

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

601f3b1Merge branch 'u/dlucas/abstract_lin_code_print_error_messages' of trac.sagemath.org:sage into abstract_lin_code_print_error_messages
98d8b0aReturns -> Return
b0bca6fPython 3 ready

comment:6 follow-up: Changed 5 years ago by dlucas

  • Keywords rd3 added
  • Status changed from needs_work to needs_review

I fixed what you asked, except for your third comment, as I'm not sure it's possible to do that.

comment:7 in reply to: ↑ 6 Changed 5 years ago by bruno

  • Status changed from needs_review to positive_review

Replying to dlucas:

I fixed what you asked, except for your third comment, as I'm not sure it's possible to do that.

Nice. One could use the attribute __name__ to get rid of the <class ...> but it wouldn't be perfect at all: one would get

RuntimeError: Please override _repr_ in the implementation of MyCode_with_category

I am not sure this is actually better. So I think the current ticket in its current form is already a sufficient improvement over the current state.

comment:8 Changed 5 years ago by vbraun

  • Branch changed from u/dlucas/abstract_lin_code_print_error_messages to b0bca6f978580b57129256264b338e6b0adb6cc1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.