Opened 11 years ago
Closed 10 years ago
#12510 closed enhancement (fixed)
Add consistency tests for __nonzero__ in TestSuite.
Reported by: | Florent Hivert | Owned by: | Florent Hivert |
---|---|---|---|
Priority: | major | Milestone: | sage-5.7 |
Component: | categories | Keywords: | |
Cc: | Christian Stump, Sage Combinat CC user | Merged in: | sage-5.7.beta1 |
Authors: | Florent Hivert | Reviewers: | Nathann Cohen |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
With many datastructure for elements, comparison to zero could be faster than
comparison of two elements. The pythonic way to do that is to use indirectly
e.__nonzero__()
for example in if not e: ...
. However, to be able
to use consistently this idiom, we have to make sure that the return value of
e == e.parent().zero()
agrees with bool(e)
. The purpose of this
patch is to add this tests to the standard test suite and to fixe the Sage
library according to this policy.
On the way, I discovered that for a some morphism of modules, pickling can be broken if the zero morphism is asked. I also fixes this issue.
See also discussion on #12508. Finally, I left to #12526 the question of removing __nonzero__
from element and writing tuned implantation for all data structures.
Attachments (1)
Change History (12)
comment:1 Changed 11 years ago by
Authors: | → Florent Hivert |
---|---|
Description: | modified (diff) |
comment:2 Changed 11 years ago by
Status: | new → needs_review |
---|
comment:3 Changed 11 years ago by
Owner: | changed from Nicolas M. Thiéry to Florent Hivert |
---|
comment:4 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 10 years ago by
The returned result was wrong so I switched back to raising not implemented. I created #13999 to have the problem fixed. Please review,
Florent
comment:7 Changed 10 years ago by
Status: | needs_review → needs_work |
---|
Changed 10 years ago by
Attachment: | trac_12510-nonzero_equal_consistency-fh.patch added |
---|
comment:8 Changed 10 years ago by
Status: | needs_work → positive_review |
---|
Gooooooooooooooooooooooood to go !!!
Le jury apprécie : la qualité des workarounds :-D
Nathnan
comment:10 Changed 10 years ago by
Reviewers: | → Nathann Cohen |
---|
comment:11 Changed 10 years ago by
Merged in: | → sage-5.7.beta1 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Helloooooooo !!
Well, as far as I understand this code I think that it is good to go, except for this thing I just told you about : the new content of
QuotientRingElement.__nonzero__
creates this problemIf you think that this is not a problem after all, then you can set this ticket to
positive_review
.Have fuuuuuuuuuuuuuuuuuuuuuuuunnn !!
Nathann