#17317: Add unit_group() method to IntegerModRing
-------------------------------------+-------------------------------------
Reporter: pbruin | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.4
Component: number theory | Resolution:
Keywords: unit group | Merged in:
Authors: Peter Bruin | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/pbruin/17317-IntegerModRing_unit_group|
7f38953bdbc38709a218533934d759abc5325f76
Dependencies: #17315 | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by jdemeyer):
* status: needs_review => needs_work
Comment:
First impressions:
1. I find the examples in the doctest all rather simple. It doesn't really
show how different the `algorithm="sage"` and `algorithm="pari"` can be. I
would add `Zmod(66)` as example. Also a larger example, say
`Zmod(lcm([1..40]))` or so...
2. I would add a `**kwds` argument to `unit_gens()`:
{{{
def unit_gens(self, **kwds):
return self.unit_group(**kwds).gens_values()
}}}
3. And a nitpick: I prefer to see `repr(algorithm)` in the error message,
something like:
{{{
raise ValueError('unknown algorithm %r for computing the unit group' %
algorithm)
}}}
This error should also be doctested in a `TESTS::` block.
--
Ticket URL: <http://trac.sagemath.org/ticket/17317#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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
