#18099: Prepare linear_code for inheritance
-------------------------------------+-------------------------------------
Reporter: dlucas | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.6
Component: coding theory | Resolution:
Keywords: sd66 | Merged in:
Authors: David Lucas | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/dlucas/prepare_linear_code_for_inheritance|
a425a89873d194bfdff1add267502a8f84ca6e08
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by ncohen):
* status: needs_review => needs_work
Comment:
Helloooooooooooooooooooooo,
Several comments:
- (superficial) The way you moved code around makes the commit much more
difficult to read than it should. It is a good thing to quickly read the
commits before you push them, as usually the reviewer has to
"reverse-engineer" what you want to do from the diff file. If you are
forced
to move code around, it is better to do so in a specific commit which
*only*
moves the code without changing anything in it.
As a reviewer, we have to make sure that all changes are correct, and as
moving code around appears to us as 1) remove the code 2) add the code
we have
to re-read and check all of it, even if it was a no-brainer change on
your
side.
- I changed your toy example a bit so that the matrix is given as
argument. I
hope that you will not mind.
- While modifying `minimum_distance()` and `gens()` you interfered with
its
caching mechanism: answers which used to be cached are now forgotten.
Could
you fix that?
- The doctest you wrote for the abstract class takes a lot of time, and we
try
to not go over a couple of seconds if we can avoid it (even for long
ones). As
this one does not test a very fundamental feature, could you make it a
bit
faster by changing the figures?
Thanks,
I added a small commit at public/18099. The usual procedure is that, as I
reviewed your changes, you can be the reviewer of mine. In particular,
feel free
to discuss/oppose any of the changes.
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/18099#comment:10>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
