#12496: Improve doctest coverage for integer_ring.pyx
-----------------------------+----------------------------------------------
Reporter: slelievre | Owner: slelievre
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-5.0
Component: documentation | Keywords: Cernay2012
Work_issues: | Upstream: N/A
Reviewer: | Author:
Merged: | Dependencies:
-----------------------------+----------------------------------------------
Changes (by hthomas):
* status: needs_review => needs_work
Comment:
Hi!
I am also pretty new to contributing to Sage, and to reviewing. So I have
some comments, but they might not all really need to be dealt with. I
guess the things that confused me might confuse other people as well, but
there's no reason for a non-expert to be looking at some of these things,
so maybe it's not really a problem. On the other hand, probably no one
will get around to touching this documentation again for a long time, so
it makes sense to try to make it as good as possible now.
There are some methods which I couldn't figure out what they were supposed
to do (mathematically speaking): degree, absolute_degree, parameter.
There are some methods for which it would be useful (I think) to explain
more clearly what the arguments are.
* gen: what is n?
* completion what is prec? (well, I figured out it was precision)
* crt_basis: what does the xgcd parameter do?
* !__getitem!__: what is x?
* _is_valid_homomorphism_: what are the arguments?
* extension: what are the arguments?
There are a few places where I wanted to change the wording:
* "to a void" (in cdef void): I think this should be "to avoid"
* "The default distribution is on average 50% `\pm 1`" (in
random_element): I'm not sure what this means. If it means "is either 1
or -1 50% of the time on average" then it contradicts the discussion
above, which says each of 1, 0, -1 occurs 20% of the time. Also, maybe it
would be good to include some link to where the reader could find out
about the available distributions.
* "Return the order in the number field defined by poly generated (as a
ring) by a root of poly." (in extension) I had trouble making sense of
this. I think something like "Given a polynomial as input, return the
order generated by a root of the polynomial in the number field defined by
the polynomial." would be clearer.
_randomize_mpz could do with more detailed documentation (I think).
_cmp_ could also do with more documentation -- I couldn't figure out what
it was doing.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12496#comment:4>
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.
