#10771: gcd and lcm for fraction fields
--------------------------------+-------------------------------------------
Reporter: SimonKing | Owner: AlexGhitza
Type: defect | Status: needs_review
Priority: major | Milestone: sage-4.7
Component: basic arithmetic | Keywords: gcd lcm fraction fields
Author: Simon King | Upstream: N/A
Reviewer: Marco Streng | Merged:
Work_issues: |
--------------------------------+-------------------------------------------
Changes (by mstreng):
* reviewer: => Marco Streng
Comment:
A couple of comments:
* I don't see the point of keeping a special function
{{{gcd_rational(self, other, **kwds):}}} that returns a GCD in the set
{0,1}. Why only for QQ? Why at all? Also, the difference between {{{gcd}}}
and {{{gcd_rational}}} is not explained by the word "rational". Perhaps
{{{gcd_zero_one}}} would be a more informative name.
* Everything generalizes from principal ideal domains to unique
factorization domains (UFD's) (such as multivariate polynomial rings over
unique factorization domains) as long as they have {{{gcd}}} and {{{lcm}}}
methods implemented. Why not write "unique factorization domain" in the
documentation instead of "principal ideal domain"?
* Suppose I have a fraction field F of a ring of type R that does not have
lcm and gcd methods, or these methods exist, but raise other kinds of
errors, e.g. because the ring is not a UFD or the methods have not been
implemented. Let a and b be elements of F. Then a and b have a gcd in F
because F is a field, so I would expect a.gcd(b) to return something
(anything basically). After applying your patch, if I do {{{a.gcd(b)}}},
it is very confusing to get an {{{AttributeError: 'RElement' object has no
attribute 'gcd'}}}: I'm not interested in gcd's of RElements, only of
{{{(Fraction)FieldElements}}}. You could put your entire {{{gcd}}} and
{{{lcm}}} code between {{{try:}}} and {{{except (AttributeError,
NotImplementedError, TypeError, ValueError):}}} to return the same 0 or 1
that {{{gcd_rational}}} would (which is a mathematically correct gcd in F
after all).
* trac has a `t` too much in line 1610 of ring.pyx: {{{the case of the
rational field. However, since tract ticket #10771,}}}
* you write "quotient field" in the documentation. You could write
"fraction field" to avoid confusion with quotient rings, which may be
fields. This makes it more clear that you refer to the mathematical
counterpart of Sage's "{{{FractionField}}}"?
* {{{All tests passed!}}} with -long, well documented, looks good.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10771#comment:3>
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.
