Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AbstractLinearCode should throw sensible error messages on printing #20899

Closed
johanrosenkilde opened this issue Jun 28, 2016 · 13 comments
Closed

Comments

@johanrosenkilde
Copy link
Contributor

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.

CC: @sagetrac-dlucas

Component: coding theory

Keywords: linear code, error messages, rd3

Author: David Lucas

Branch/Commit: b0bca6f

Reviewer: Bruno Grenet

Issue created by migration from https://trac.sagemath.org/ticket/20899

@johanrosenkilde
Copy link
Contributor Author

comment:1

Fixed amusing spelling mistake in the title...

@johanrosenkilde johanrosenkilde changed the title AbstractLinearCode shouldn't throw sensible error messages on printing AbstractLinearCode should throw sensible error messages on printing Jun 29, 2016
@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jun 29, 2016

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jun 29, 2016

Commit: 152c471

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jun 29, 2016

comment:3

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

@bgrenet
Copy link
Contributor

bgrenet commented Feb 7, 2017

Author: David Lucas

@bgrenet
Copy link
Contributor

bgrenet commented Feb 7, 2017

Reviewer: Bruno Grenet

@bgrenet
Copy link
Contributor

bgrenet commented Feb 7, 2017

comment:4

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'>

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2017

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2017

Changed commit from 152c471 to b0bca6f

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

comment:6

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

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

Changed keywords from linear code, error messages to linear code, error messages, rd3

@bgrenet
Copy link
Contributor

bgrenet commented Feb 7, 2017

comment:7

Replying to @sagetrac-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.

@vbraun
Copy link
Member

vbraun commented Feb 8, 2017

Changed branch from u/dlucas/abstract_lin_code_print_error_messages to b0bca6f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants