#7857: Arithmetic operations in fraction fields
---------------------------+------------------------------------------------
Reporter: spancratz | Owner: spancratz
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.3.1
Component: algebra | Keywords: fraction field
Work_issues: | Author: spancratz
Upstream: N/A | Reviewer: John Cremona
Merged: |
---------------------------+------------------------------------------------
Comment(by spancratz):
Dear John,
Thank you for looking at the patch this quickly.
About your first point, I agree. I followed the previous code of the
method "reduce" in this part, which is why I am not completely sure I'll
be able to find out exactly which exceptions might be raised. But I will
look into this.
I think you are also right with your second point. Of course, when I
wrote the code I simply copied the method and replaced the appropriate
signs to avoid the unnecessary overhead, but in this context this is
probably a bad habit, since the maintainability of the code should take
priority over a simple sign swap plus an extra function call. By the way,
the same applies to multiplication and division, too.
I'll aim to complete this tonight or tomorrow.
Kind regards,
Sebastian
Replying to [comment:3 cremona]:
> First comment: Note that the "second patch" replaces the first one!
>
> I noticed a bare "except:" which is bad style -- it should list the
errors expected explicitly.
>
> Question: the code for sub is (of course) almost identical to that for
add. Would it not be better to implement sub by adding the inverse? I
would have thought that the overhead would be trivial, and itmakes the
code easier to maintain.
>
> The (second) patch applies fine to 4.3 and all tests in sage/rings and
sage/schemes/elliptic_curves pass. (I tested the latter since I seemed to
remember some fraction field arithmetic somewhere in there). I also
checked the docs still build ok.
>
> I'll tag this as "needs work" but it's very minor to fix the exception
trapping and (perhaps) replace the subtraction code.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7857#comment:6>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
