#8200: ElementWrapper: doctests improvements to not abuse ZZ invariants
-------------------------------------------------+--------------------------
Reporter: nthiery | Owner: was
Type: defect | Status: needs_work
Priority: blocker | Milestone: sage-4.3.3
Component: doctest | Keywords: ElementWrapper
Author: Florent Hivert, Nicolas M. Thiéry | Upstream: N/A
Reviewer: William Stein | Merged:
Work_issues: |
-------------------------------------------------+--------------------------
Changes (by nthiery):
* status: positive_review => needs_work
* reviewer: Florent Hivert => William Stein
Old description:
> The attached patch updates the doctests of ElementWrapper to use a custom
> dummy parent, rather than abusing from ZZ. This abuse could trigger a
> segfault (see #8177).
New description:
The attached patch updates the doctests of ElementWrapper to use a custom
dummy parent, rather than abusing from ZZ. This abuse could trigger a
segfault (see #8177).
I put this back to "needs review", *without* the reviewer's patch.
--
Comment:
Replying to [comment:5 was]:
> This patch itself looks fine and passes tests.
Thanks for the review.
> However, it's critical that a patch to address this problem at least
include a comment that completely explains the problem. As it is, this
patch just hides a problem that will certainly cause similar great
confusion and pain later.
I fully agree that the issue with ZZ/Integer should not be left hidden,
which is precisely why I forked this ticket in two.
But sorry, I don't accept the reviewer's patch. Integer/ZZ is broken.
That's the purpose of #8177. Abusing ElementWrapper with ZZ as parent was
broken. That's the purpose of this ticket. But ElementWrapper in itself is
*not* broken (well at least in this context). Any other Sage element class
that accepts a parent as input (that is most of them) would trigger the
exact same issue when abused the same way.
I don't want people to be scared away of ElementWrapper because of an
issue elsewhere.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/8200#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.
