#13687: Parents for groups
--------------------------------+-------------------------------------------
Reporter: vbraun | Owner: joyner
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-5.5
Component: group theory | Resolution:
Keywords: new Parent | Work issues:
Report Upstream: N/A | Reviewers: David Roe
Authors: Volker Braun | Merged in:
Dependencies: | Stopgaps:
--------------------------------+-------------------------------------------
Changes (by roed):
* status: needs_review => needs_work
* reviewer: => David Roe
Comment:
Hi Volker,
I'm about halfway through this. Overall it's looking great. Here are
some comments (let me know if anything is unclear):
* in the `_normalize` function for `AbelianGroup`, perhaps the 0s should
come last, since that lines up with the standard divisibility condition
for invariants. Also, if `gens_orders` is a string of length `n` then it
will get through your error checking.
`_normalize` should have examples.
* In `random_element`, shouldn't the fact that doctesting consistently
chooses a random seed mean we don't need to mark `G.random_element()` as
`# random output`?
* In the docstring for `equals`, the documentation for the output is
wrong: should be checking whether they are the same subset.
* In `invs()`, where you write `raise Exception # WTF`, maybe we should
figure out what was intended by this function and fix it. :-)
* In the module summary of `dual_abelian_group`, you can use floating
point tolerance rather than just marking the output as random. In
particular, shouldn't your first example be exact?
* In module doctoring for `dual_abelian_group_element`, you should take
out the `# random` marking on some doctests (using floating point
tolerance instead if necessary)
I should go to sleep, but I'll try to get through more of this tomorrow.
When you make changes, could you upload a separate patch?
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13687#comment:5>
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.
