Opened 14 years ago

Closed 14 years ago

#911 closed defect (fixed)

[with patch] hash() on Graph objects changes as the object is mutated

Reported by: cwitty Owned by: was
Priority: major Milestone: sage-2.8.8
Component: combinatorics Keywords: graphs
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

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?.

Attachments (1)

graph_hash.patch (1.0 KB) - added by ekirkman 14 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 14 years ago by jason

  • Component changed from basic arithmetic to combinatorics
  • Keywords graphs added
  • Milestone changed from sage-2.9 to sage-2.8.8
  • Owner changed from somebody to was
  • Summary changed from hash() on Graph objects changes as the object is mutated to [with patch] hash() on Graph objects changes as the object is mutated

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):
         """

Changed 14 years ago by ekirkman

comment:2 Changed 14 years ago by ekirkman

  • Summary changed from [with patch] hash() on Graph objects changes as the object is mutated to [with patch][tested by ekirkman] hash() on Graph objects changes as the object is mutated

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

comment:3 Changed 14 years ago by mabshoff

  • Summary changed from [with patch][tested by ekirkman] hash() on Graph objects changes as the object is mutated to [with patch] hash() on Graph objects changes as the object is mutated

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

[tested by ...]

byline.

Cheers,

Michael

comment:4 Changed 14 years ago by was

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.