#13412: PowerSeriesRing should call Ring.__init__
----------------------------------------------------+-----------------------
Reporter: SimonKing | Owner: nthiery
Type: defect | Status:
needs_review
Priority: major | Milestone: sage-5.4
Component: categories | Resolution:
Keywords: power series, coercion, category | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Simon King | Merged in:
Dependencies: | Stopgaps:
----------------------------------------------------+-----------------------
Comment (by SimonKing):
Replying to [comment:10 nthiery]:
> Near the TestSuite calls, I see things like:
> {{{
> sage: loads(dumps(X)) is X
> True
> }}}
> This is a generic test, so I'd rather not include those, and if you feel
strong about them to add it as a generic test in the TestSuite.
I added it near the `TestSuite` calls, ''because'' it looks like a generic
test (I argue below that it isn't really generic), and because the
`TestSuite` does not (and can not, I think) cover it.
> Maybe the X._test_pickling() should check whether X has unique
representation, and test with `is` or `==` accordingly?
> Or instead one could just check elsewhere that equality is indeed
defined in term of ``is`` (like testting whether the __eq__ method is that
inherited from UniqueRepresentation)
I thought that to `TestSuite(X)` belong all generic tests that only depend
on the category of X. But apparently `X.category()` will not tell whether
X is a unique parent.
Moreover, doing `if isinstance(self, UniqueRepresentation)` or testing the
`__eq__` method would certainly not cover all unique parents. Namely some
of them are created by a `UniqueFactory`. In that case, neither the
category nor the class give a hint on whether pickling should work with `X
is loads(dumps(X))`.
Just think of polynomial rings: They are unique parents, but that's
because of a polynomial ring constructor, and they do have
{{{
cdef int _cmp_c_impl(left, Parent right) except -2:
if isinstance(right, (MPolynomialRing_libsingular,
MPolynomialRing_polydict_domain)):
return cmp((left.base_ring(), map(str, left.gens()),
left.term_order()),
(right.base_ring(), map(str, right.gens()),
right.term_order()))
else:
return cmp(type(left),type(right))
}}}
The `X is loads(dumps(X))` thus ''not'' being generic, I think it clearly
does not belong to `TestSuite(X)`. I do feel strongly about having that
test in addition `TestSuite(X)`.
> line 362: this should be {{{is_MPowerSeriesRing(S)}}}:
> {{{
> if (is_MPolynomialRing(S) or is_MPowerSeriesRing)
> }}}
Right. And there should obviously be a test where `is_MPowerSeriesRing(S)`
is needed.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13412#comment:11>
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.
