#10771: gcd and lcm for fraction fields
--------------------------------+-------------------------------------------
Reporter: SimonKing | Owner: AlexGhitza
Type: defect | Status: needs_work
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: |
--------------------------------+-------------------------------------------
Comment(by SimonKing):
Replying to [comment:13 mstreng]:
> * Did you run a full doctest? I'm getting doctest failures that may
have something to do with your patch, as they probably simplify fractions
somewhere deep inside in some way. Or my installation is messed up again.
> {{{
> sage -t -long devel/sage/sage/symbolic/expression.pyx # 1 doctests
failed
> sage -t -long devel/sage/sage/symbolic/integration/integral.py # 1
doctests failed
> sage -t -long devel/sage/sage/stats/basic_stats.py # 2 doctests failed
> }}}
Hum. Apparently I did not run these tests. The problem is that I currently
work on various patches, frequently switch between them, and it may happen
that I test one of them but forget the other.
Anyway, the failure in "expression.pyx" seems to be related with the
patch. I'll try to understand what is happening.
> * The documentation of content for multivariate polynomials says
"Returns the content of this polynomial.
> Here, we define content as the gcd of the coefficients in the base
ring."
> Your changed doctest (1 becomes 2) is correct, but perhaps it would be
more informative to
> add {{{f.content().parent()}}}, and to give an example over another
field. (Just a suggestion
> if you are editing the code anyway. Otherwise, don't bother.)
OK.
> More important:
> {{{
> # Since trac ticket #10771, the gcd in QQ restricts to the
> # gcd in ZZ.
> }}}
> I don't think this comment is needed in this part of the code. If you
want to include it, it would better fit in the doctest a few lines above
it.
I thought of it as a comment to people who know the original code. It
could be that it is a method that uses gcd only internally, so that
commenting in the code seems better than in the documentation.
> And I think junk like the following should not be in the code at all:
{{{ #,integer=self.parent() is ZZ) }}}
OK, that was an artefact of editing.
But I think most urgent are the doctest failures. It seems related to gcd
for fields that are not fraction fields (such as `RR`). That would also
explain why all text passed with the original version of my patch, whereas
the new version fails.
Cheers,
Simon
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10771#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 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.
