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

hash() on Graph objects changes as the object is mutated #911

Closed
sagetrac-cwitty mannequin opened this issue Oct 17, 2007 · 4 comments
Closed

hash() on Graph objects changes as the object is mutated #911

sagetrac-cwitty mannequin opened this issue Oct 17, 2007 · 4 comments

Comments

@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Oct 17, 2007

This is bad:

sage: foo = Graph()
sage: hash(foo)
1033452963
sage: foo.add_vertex(1)
sage: hash(foo)
1537218837

__hash__ on Graph objects should be overridden to raise a TypeError.

Component: combinatorics

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

@sagetrac-cwitty sagetrac-cwitty mannequin added this to the sage-2.9 milestone Oct 17, 2007
@jasongrout
Copy link
Member

comment:1

From the python docs for __hash__ at http://docs.python.org/ref/customization.html :

"If a class defines mutable objects and implements a cmp() or eq() method, it should not implement hash(), since the dictionary implementation requires that a key's hash value is immutable (if the object's hash value changes, it will be in the wrong hash bucket)."

Currently, a Graph object defines a __cmp__ method, but not a __hash__ method. This seems to be in accordance with the python docs. However, I guess we are inheriting the __hash__ method from SageObject, so we should redefine the __hash__ method?

Here's a patch:

diff -r 36489d2c9a2e sage/graphs/graph.py
--- a/sage/graphs/graph.py      Tue Oct 16 09:50:59 2007 -0500
+++ b/sage/graphs/graph.py      Wed Oct 17 10:19:53 2007 -0500
@@ -359,6 +359,20 @@ class GenericGraph(SageObject):
             return self._nxg.name
         else:
             return repr(self)
+
+    def __hash__(self):
+        """
+       Since graphs are mutable, they should not be hashable, so we return a type error.
+
+       EXAMPLE:
+           sage: hash(Graph())
+            Traceback (most recent call last):
+            ...
+            TypeError: graphs are mutable, and so therefore are unhashable
+    
+       """
+       raise TypeError, "graphs are mutable, and so therefore are unhashable"
+
 
     def _latex_(self):
         """

@sagetrac-ekirkman
Copy link
Mannequin

sagetrac-ekirkman mannequin commented Oct 18, 2007

Attachment: graph_hash.patch.gz

@sagetrac-ekirkman
Copy link
Mannequin

sagetrac-ekirkman mannequin commented Oct 18, 2007

comment:2

Defect resolved by attached patch. Ready to include in 2.8.8!

@sagetrac-ekirkman sagetrac-ekirkman mannequin changed the title hash() on Graph objects changes as the object is mutated [tested by ekirkman] hash() on Graph objects changes as the object is mutated Oct 18, 2007
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Oct 18, 2007

comment:3

Please see http://groups.google.com/group/sage-devel/t/72ca87d027fb3e63 regarding the

[tested by ...]

byline.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin changed the title [tested by ekirkman] hash() on Graph objects changes as the object is mutated hash() on Graph objects changes as the object is mutated Oct 18, 2007
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

2 participants