#13687: Parents for groups
--------------------------------+-------------------------------------------
Reporter: vbraun | Owner: joyner
Type: enhancement | Status: needs_review
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:
--------------------------------+-------------------------------------------
Comment (by roed):
Here are some more suggestions. I'm going to leave it marked as needs
review so that the patchbot will continue testing.
1. In `AbelianGroupElement.__init__`, I think it's useful to allow `ints`
in the `generator_orders`, rather than asserting that they all be
`Integers`
2. Documentation of `exponents` doesn't line up with `__init__` method in
`AbelianGroupElementBase`. You should give an example in the
documentation of that function showing that the exponents are still
integers, not elements of the base ring.
3. In docstring for `AbelianGroupWithValues`, maybe you should note that
`values_group` could be a ring or field, since Sage doesn't currently
support the group of units in a ring very well.
4. I don't think that `AbelianGroupWithValuesEmbedding` should inherit
from `CallMorphism`, but rather just from `Morphism`.
5. For the arithmetic operations in `AbelianGroupWithValuesElement`, I
would keep the computation of `_value` lazy (check to see if both left and
right values are not None). By being nonlazy you'll get more floating
point drift and also you may compute unnecessary values.
6. Your docstring for `AbelianGroupWithValues_class.values_group` is
incomplete.
7. Indentation problem at the end of the docstring for `Group` (right
before `def __init__`)
8. You set `Element = FractionalIdealClass` on
`SFractionalIdealClass(FractionalIdealClass)`. This should be done on the
parent, not the element.
9. There doesn't seem to be a failure in the test suite for `ClassGroup`,
so you should remove the sentence about a failure caused by not having a
category of additive abelian groups.
10. In `unit_group.py` you use `_.value()` a few times; can you give
`UK.torsion_generator()` (etc) a name and use it instead of `_`?
11. You have a docstring floating that needs to be deleted where
`gen(self, i=0)` used to be in `UnitGroup`
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13687#comment:9>
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.
