#11836: gens_reduced() does not handle "large" ideals
-----------------------------+----------------------------------------------
Reporter: mirela | Owner: jdemeyer
Type: defect | Status: needs_review
Priority: major | Milestone: sage-4.7.2
Component: number fields | Keywords:
Work_issues: | Upstream: N/A
Reviewer: Marco Streng | Author: Jeroen Demeyer
Merged: | Dependencies: #11130
-----------------------------+----------------------------------------------
Comment(by mstreng):
Looking at the patch right now. Passed all tests, building documentation.
Question now that I'm making comments anyway: between lines 1060 and 1061,
should/could you add {{{self.__is_principal = ...}}}? Just in case some
method comes into existence later that sets {{{__ideal_class_log}}}
without setting {{{__is_principal}}}.
{{{
sage: K.<a> = QuadraticField(-5)
sage: P = K.ideal(3).factor()[0][0]
sage: P.is_principal()
False
sage: P = K.ideal(3).factor()[0][0]
sage: P.__ideal_class_log = [1]
sage: P.is_principal()
AttributeError: 'NumberFieldFractionalIdeal' object has no attribute
'_NumberFieldIdeal__is_principal'
}}}
Also, the use of v[1] in 1073 requires some complicated logic to see that
it is really defined. I think it could lead to bugs when other people edit
your code later without realising this.
(p.s. I'm not setting anything to "needs work" for any of this.)
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11836#comment:14>
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.
