Opened 6 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: |
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 6 years ago by
- Summary changed from `AbstractLinearCode` shouldn't throw sensible error messages on printing to `AbstractLinearCode` should throw sensible error messages on printing
comment:2 Changed 6 years ago by
- Branch set to u/dlucas/abstract_lin_code_print_error_messages
comment:3 Changed 6 years ago by
- 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 ofNotImplementedError
because usingNotImplementedError
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:
4862c83 | Moved _latex_ method from AbstractLinearCode to LinearCode
|
152c471 | Added generic _repr_ and _latex_ which raise informative error messages
|
comment:4 Changed 5 years ago by
- 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 beReturn
(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
instead of
RuntimeError: Please override _repr_ in the implementation of MyCode
RuntimeError: Please override _repr_ in the implementation of <class __main__.MyCode_with_category'>
comment:5 Changed 5 years ago by
- Commit changed from 152c4711a4ca9c79878129cb0b5881e1c315d4cb to b0bca6f978580b57129256264b338e6b0adb6cc1
comment:6 follow-up: ↓ 7 Changed 5 years ago by
- 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
- 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
- Branch changed from u/dlucas/abstract_lin_code_print_error_messages to b0bca6f978580b57129256264b338e6b0adb6cc1
- Resolution set to fixed
- Status changed from positive_review to closed
Fixed amusing spelling mistake in the title...