#16250: Monoids._test_one() incorrect for unhashable one elements
-------------------------------------+-------------------------------------
Reporter: saraedum | Owner:
Type: defect | Status: needs_review
Priority: trivial | Milestone: sage-6.3
Component: categories | Resolution:
Keywords: hash | Merged in:
Authors: Julian Rüth | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/saraedum/ticket/16250 | 7b35c8a32ea67041665dddd8f1d98c7b8cfb6141
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by nthiery):
Hi!
Thanks for shaking the generic tests with one more use case :-)
I agree that it makes sense to disable the test if the object is not
hashable. On the other hand, I would much prefer to have an explicit
test like:
{{{
isinstance(..., collections.Hashable)
}}}
This better expresses the intention and also avoids the risk of false
positives in case a TypeError failure would be raised for some other
reasons and be silently caught.
Of course the above isinstance test fails if there is a `__hash__`
method defined. But this can be conveniently fixed by setting
`__hash__` to `None` in the element class. In fact this sounds like a
natural thing to do anyway: having `__hash__` defined and return
something, and worst something that could change over time, is nothing
but a potential source of issues, isn't it?
With `__hash__` set to `None`, we have as desired:
{{{
sage: import collections
sage: K.<a> = Qq(9)
sage: K.zero
sage: isinstance(K.zero(), collections.Hashable)
False
}}}
Agreed, the right fix (tm) would probably be to remove the infamous
`SageObject.__hash__` method in the first place. Not all SageObjects
are hashable, and anyway computing the hash in term of `_repr_` is
just a continuous source of bugs. But this fix could take some work
since one would need to implement `__hash__` properly in a bunch of
places; so let's say that this is for a separate ticket.
Cheers,
Nicolas
--
Ticket URL: <http://trac.sagemath.org/ticket/16250#comment:6>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
