#9094: is_square and sqrt for polynomials and fraction fields
------------------------------------------------+---------------------------
Reporter: robertwb | Owner: AlexGhitza
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.5.1
Component: algebra | Keywords:
Author: Robert Bradshaw, Maarten Derickx | Upstream: N/A
Reviewer: John Cremona, Marco Streng | Merged:
Work_issues: documentation, double code |
------------------------------------------------+---------------------------
Changes (by newvalueoldvalue):
* status: needs_review => needs_work
* reviewer: mstreng => John Cremona, Marco Streng
* work_issues: improve documentation, add more tests => documentation,
double code
* author: mderickx => Robert Bradshaw, Maarten Derickx
Comment:
fraction_field_element.pyx, line 326 "_parent" --> "parent()" (don't use a
hidden variable, use the function that returns its value)
polynomial_ring_element.pyx, line 1306, "#This code is quite general, it
works for all integral domains that have the is_square(root = True)
option"
- This comment is confusing: does "all integral domains that..." refer to
self.parent() or to self.parent().base_ring()? Why not just remove this
comment?
- Are you proposing to move the code to are more general class of ring
elements? Then why not just do that? Right now, you have the same code for
fraction fields and polynomial rings, but you could just put it in a more
general class and add a test if self.parent() is a domain and
"self.is_square()" exists and has a parameter "root". (You don't have to
do this to get a positive review, but you can think about it and/or try
it)
(for both files:) It is helpful for others if the documentation of sqrt
says how square roots are computed, i.e. says "Calls is_square(root =
True) to find the square root". This can be done in an "ALGORITHM:" block.
Build the documentation using "sage -docbuild reference html" to test the
following yourself:
- the input, output, and examples blocks of lines 1260, 1265, and 1269 of
sqrt for Polynomial don't indent and don't format nicely. All this needs
is an extra empty line after the mentioned lines. See for example the
is_square documentation.
- same for the input and output blocks of sqrt for fraction field elements
on lines 336 and 341.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9094#comment:13>
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.
