#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|
8905552948a0896a46e587815cce539b6e60aae9
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by dlucas):
Replying to [comment:10 ncohen]:
> Helloooooooooooooooooooooo,
Hello!
> 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.
Got it! Thanks for the advice :)
> - I changed your toy example a bit so that the matrix is given as
argument. I
> hope that you will not mind.
Not at all. I kept it in the new version of the code.
> - 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?
Done. These methods are now cached.
> - 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?
Sure. Considering I picked some random methods to illustrate the
inheritance mechanism with a dummy class, I just replaced it by something
faster.
David
--
Ticket URL: <http://trac.sagemath.org/ticket/18099#comment:14>
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.
