Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add consistency tests for __nonzero__ in TestSuite. #12510

Closed
hivert opened this issue Feb 15, 2012 · 9 comments
Closed

Add consistency tests for __nonzero__ in TestSuite. #12510

hivert opened this issue Feb 15, 2012 · 9 comments

Comments

@hivert
Copy link
Contributor

hivert commented Feb 15, 2012

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.

CC: @stumpc5 @sagetrac-sage-combinat

Component: categories

Author: Florent Hivert

Reviewer: Nathann Cohen

Merged: sage-5.7.beta1

Issue created by migration from https://trac.sagemath.org/ticket/12510

@hivert hivert added this to the sage-5.7 milestone Feb 15, 2012
@hivert

This comment has been minimized.

@hivert
Copy link
Contributor Author

hivert commented Feb 17, 2012

Author: Florent Hivert

@hivert hivert assigned hivert and unassigned nthiery Feb 17, 2012
@hivert

This comment has been minimized.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 21, 2013

comment:5

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 problem

sage: a = e.lift()
sage: any([e])
True
sage: b = e.lift()
# Now b and a are not necessarily equal anymore

If you think that this is not a problem after all, then you can set this ticket to positive_review.

Have fuuuuuuuuuuuuuuuuuuuuuuuunnn !!

Nathann

@hivert
Copy link
Contributor Author

hivert commented Jan 24, 2013

comment:6

The returned result was wrong so I switched back to raising not implemented. I created #13999 to have the problem fixed. Please review,

Florent

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 24, 2013

comment:8

Attachment: trac_12510-nonzero_equal_consistency-fh.patch.gz

Gooooooooooooooooooooooood to go !!!

Le jury apprécie : la qualité des workarounds :-D

Nathnan

@hivert
Copy link
Contributor Author

hivert commented Jan 24, 2013

comment:9

Thanks Nathan !!!

@jdemeyer
Copy link
Contributor

Reviewer: Nathann Cohen

@jdemeyer
Copy link
Contributor

Merged: sage-5.7.beta1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants