On Mon, Apr 18, 2011 at 9:09 PM, Robert Bradshaw
<[email protected]> wrote:
> On Mon, Apr 18, 2011 at 4:13 PM, William Stein <[email protected]> wrote:
>> Hi,
>>
>> I was just explaining to a student in my Sage course how I had
>> stupidly defined a default __hash__ method for SageObject, which was
>> -- stupidly -- to just hash the string representation. This was of
>> course silly and dangerous since only immutable objects should have a
>> __hash__ method. I then of course pointed out that we must have
>> fixed this years ago! But.... amazingly we didn't:
>>
>> sage: S = SageObject(); S
>> <type 'sage.structure.sage_object.SageObject'>
>> sage: hash(S)
>> -7904861314369208036
>> sage: S.__hash__??
>> File: /sagenb/flask/sage-4.6.2/devel/sage/sage/structure/sage_object.pyx
>> Source Code (starting at line 156):
>> def __hash__(self):
>> return hash(self.__repr__())
>
> By convention, almost all SageObjects are immutable, so it was
> probably easier to provide a default and override it for exceptions
> (matrices and vectors are all that come to mind) than implement it for
> every concrete subclass.
> - Robert
The statement "... so it was probably easier to provide a default and
override it for exceptions (matrices and vectors are all that come to
mind) than implement it for every concrete subclass." is indeed why I
implemented __hash__ that way, long ago. However, I didn't do it
because by convention almost all SageObjects are immutable. In fact,
many of them were not immutable in the beginning, and later became
immutable when I learned the extent to which mutable hashable break
basic assumptions in Python (and rightfully so).
I think it is worth revisiting this design decision. I did a
little poking, and in Python and Cython, a class by default has a
hash, which is the location in memory of the object, e.g.,
sage: class Foo:
....: pass
....:
sage: hash(f)
140309021077656
sage: id(f)
140309021077656
So the main issue I pointed out above, namely that SageObject has a
__hash__ defined, is really a non-issue, since that's the standard
default convention in Python anyways. The real issue to consider is
whether using the string representation as a default is a good idea.
The only real drawback is efficiency, since computing the string
representation of an object can be costly. So...
Project: comment out the two lines in sage_object.pyx, run the Sage
test suite, observe failures [1], choose one, and write a better
(faster; more intelligent) __hash__ method for the objects that lead
to the failure.
-- William
[1] The files in devel/sage/sage that fail are:
The following tests failed:
sage -t devel/sage-main/sage/plot/plot3d/base.pyx # 1 doctests failed
sage -t devel/sage-main/sage/schemes/generic/toric_variety.py
# 4 doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/multi_polynomial_ideal.py # 2
doctests failed
sage -t
devel/sage-main/sage/schemes/elliptic_curves/heegner.py # 2 doctests
failed
sage -t devel/sage-main/sage/interfaces/maxima.py # 4 doctests failed
sage -t devel/sage-main/sage/categories/pushout.py # 3 doctests failed
sage -t
devel/sage-main/sage/schemes/elliptic_curves/ell_curve_isogeny.py # 8
doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/polynomial_element.pyx # 2
doctests failed
sage -t devel/sage-main/sage/crypto/mq/mpolynomialsystem.py #
1 doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/multi_polynomial_sequence.py # 1
doctests failed
sage -t devel/sage-main/sage/combinat/yang_baxter_graph.py #
1 doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/multi_polynomial_libsingular.pyx
# 3 doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/multi_polynomial.pyx # 2
doctests failed
sage -t devel/sage-main/sage/tests/startup.py # 1 doctests failed
sage -t devel/sage-main/sage/categories/functor.pyx # 1 doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/infinite_polynomial_ring.py # 6
doctests failed
sage -t
devel/sage-main/sage/sets/disjoint_union_enumerated_sets.py # 10
doctests failed
sage -t devel/sage-main/sage/schemes/plane_curves/curve.py #
1 doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/laurent_polynomial_ring.py # 4
doctests failed
sage -t
devel/sage-main/sage/rings/polynomial/polynomial_ring_constructor.py #
2 doctests failed
sage -t
devel/sage-main/sage/combinat/root_system/type_relabel.py # 2 doctests
failed
--
To post to this group, send an email to [email protected]
To unsubscribe from this group, send an email to
[email protected]
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org
