trac #33255: review commit

src/sage/graphs/bipartite_graph.py

@@ -94,9 +94,9 @@ class BipartiteGraph(Graph):
     - ``weighted`` -- boolean (default: ``None``); whether graph thinks of
       itself as weighted or not. See ``self.weighted()``
 
-    - ``hash_labels`` - boolean (default: ``False``); whether to include labels
-      / weights during hashing. Will raise a warning when __hash__ is invoked
-      and default to true.
+    - ``hash_labels`` -- boolean (default: ``None``); whether to include edge
+      labels during hashing. This parameter defaults to ``True`` if the graph is
+      weighted. This parameter is ignored if the graph is mutable.
 
     .. NOTE::
 
@@ -411,7 +411,7 @@ def __init__(self, data=None, partition=None, check=True, hash_labels=None, *arg
         self.add_edges = MethodType(Graph.add_edges, self)
         alist_file = True
 
-        self.hash_labels=hash_labels
+        self._hash_labels = hash_labels
 
         from sage.structure.element import is_Matrix
         if isinstance(data, BipartiteGraph):
@@ -550,30 +550,45 @@ def __init__(self, data=None, partition=None, check=True, hash_labels=None, *arg
 
         return
 
+    @cached_method
     def __hash__(self):
-
         """
         Compute a hash for ``self``, if ``self`` is immutable.
-        """
-        if self.is_immutable():
 
-            # determine whether to hash labels
-            # warn user if not manually specified
-            use_labels=self._use_hash_labels()
+        EXAMPLES::
 
+            sage: A = BipartiteGraph([(0, 1, 1), (1, 2, 1)], immutable=True)
+            sage: B = BipartiteGraph([(0, 1, 1), (1, 2, 33)], immutable=True)
+            sage: A.__hash__() == B.__hash__()
+            True
+            sage: A = BipartiteGraph([(0, 1, 1), (1, 2, 1)], immutable=True, hash_labels=True)
+            sage: B = BipartiteGraph([(0, 1, 1), (1, 2, 33)], immutable=True, hash_labels=True)
+            sage: A.__hash__() == B.__hash__()
+            False
+            sage: A = BipartiteGraph([(0, 1, 1), (1, 2, 1)], immutable=True, weighted=True)
+            sage: B = BipartiteGraph([(0, 1, 1), (1, 2, 33)], immutable=True, weighted=True)
+            sage: A.__hash__() == B.__hash__()
+            False
 
-
-            edge_iter = self.edge_iterator(labels=use_labels)
-
+        TESTS::
+
+            sage: A = BipartiteGraph([(0, 1, 1), (1, 2, 1)], immutable=False)
+            sage: A.__hash__()
+            Traceback (most recent call last):
+            ...
+            TypeError: This graph is mutable, and thus not hashable. Create an immutable copy by `g.copy(immutable=True)`
+        """
+        if self.is_immutable():
+            # Determine whether to hash edge labels
+            use_labels = self._use_hash_labels()
+            edge_items = self.edge_iterator(labels=use_labels)
             if self.allows_multiple_edges():
-                from collections import Counter                
-                edge_items = Counter(edge_iter).items()
-            else:
-                edge_items = edge_iter
+                from collections import Counter  
+                edge_items = Counter(edge_items).items()
+            return hash((frozenset(self.left), frozenset(self.right), frozenset(edge_items)))
 
-            return hash((frozenset(self.left), frozenset(self.right), frozenset(edge_items)))        
-        else:
-            raise TypeError("This graph is mutable, and thus not hashable. Create an immutable copy by `g.copy(immutable=True)`")
+        raise TypeError("This graph is mutable, and thus not hashable. "
+                        "Create an immutable copy by `g.copy(immutable=True)`")
 
     def _upgrade_from_graph(self):
         """

src/sage/graphs/digraph.py

@@ -315,6 +315,10 @@ class DiGraph(GenericGraph):
       immutable digraph. Note that ``immutable=True`` is actually a shortcut for
       ``data_structure='static_sparse'``.
 
+    - ``hash_labels`` -- boolean (default: ``None``); whether to include edge
+      labels during hashing. This parameter defaults to ``True`` if the digraph
+      is weighted. This parameter is ignored if the digraph is mutable.
+
     - ``vertex_labels`` -- boolean (default: ``True``); whether to allow any
       object as a vertex (slower), or only the integers `0,...,n-1`, where `n`
       is the number of vertices.
@@ -855,9 +859,10 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
             self._backend = ib
             self._immutable = True
 
-        self.hash_labels=hash_labels
+        self._hash_labels = hash_labels
 
     # Formats
+
     def dig6_string(self):
         r"""
         Return the ``dig6`` representation of the digraph as an ASCII string.

src/sage/graphs/generic_graph.py

@@ -615,12 +615,39 @@ def __eq__(self, other):
 
         return self._backend.is_subgraph(other._backend, self, ignore_labels=not self.weighted())
 
-    # check if specified by the user, if not then fallback
+
     def _use_labels_for_hash(self):
-        if not hasattr(self, "hash_labels") or self.hash_labels is None:
-            fallback=self.weighted()
-            self.hash_labels=fallback
-        return self.hash_labels
+        r"""
+        Helper method for method ``__hash__``.
+
+        This method checks whether parameter ``hash_labels`` has been specified
+        by the user. Otherwise, defaults to the value of parameter ``weigthed``.
+
+        TESTS::
+
+            sage: G = Graph()
+            sage: G._use_labels_for_hash()
+            False
+            sage: G = Graph(hash_labels=True)
+            sage: G._use_labels_for_hash()
+            True
+            sage: G = Graph(hash_labels=False)
+            sage: G._use_labels_for_hash()
+            False
+            sage: G = Graph(weighted=True)
+            sage: G._use_labels_for_hash()
+            True
+            sage: G = Graph(weighted=False)
+            sage: G._use_labels_for_hash()
+            False
+            sage: G = Graph(hash_labels=False, weighted=True)
+            sage: G._use_labels_for_hash()
+            False
+        """
+        if not hasattr(self, "_hash_labels") or self._hash_labels is None:
+            self._hash_labels = self.weighted()
+        return self._hash_labels
+
 
     @cached_method
     def __hash__(self):
@@ -692,12 +719,28 @@ def __hash__(self):
             sage: G1.__hash__() == G2.__hash__()
             True
 
-        Make sure hash_labels parameter behaves as expected:
-
+        Make sure ``hash_labels`` parameter behaves as expected
+        (:trac:`33255`)::
 
+            sage: A = Graph([(1, 2, 1)], immutable=True)
+            sage: B = Graph([(1, 2, 33)], immutable=True)
+            sage: A.__hash__() == B.__hash__()
+            True
+            sage: A = Graph([(1, 2, 1)], immutable=True, hash_labels=True)
+            sage: B = Graph([(1, 2, 33)], immutable=True, hash_labels=True)
+            sage: A.__hash__() == B.__hash__()
+            False
+            sage: A = Graph([(1, 2, 1)], immutable=True, weighted=True)
+            sage: B = Graph([(1, 2, 33)], immutable=True, weighted=True)
+            sage: A.__hash__() == B.__hash__()
+            False
+            sage: A = Graph([(1, 2, 1)], immutable=True, hash_labels=False, weighted=True)
+            sage: B = Graph([(1, 2, 33)], immutable=True, hash_labels=False, weighted=True)
+            sage: A.__hash__() == B.__hash__()
+            True
         """
         if self.is_immutable():
-            use_labels=self._use_labels_for_hash()
+            use_labels = self._use_labels_for_hash()
             edge_items = self.edge_iterator(labels=use_labels)
             if self.allows_multiple_edges():
                 from collections import Counter
@@ -996,6 +1039,10 @@ def copy(self, weighted=None, data_structure=None, sparse=None, immutable=None,
             used to copy an immutable graph, the data structure used is
             ``"sparse"`` unless anything else is specified.
 
+        - ``hash_labels`` -- boolean (default: ``None``); whether to include
+          edge labels during hashing. This parameter defaults to ``True`` if the
+          graph is weighted. This parameter is ignored if the graph is mutable.
+
         .. NOTE::
 
             If the graph uses
@@ -1229,7 +1276,7 @@ def copy(self, weighted=None, data_structure=None, sparse=None, immutable=None,
             desired_immutable = self.is_immutable() if immutable is None else immutable
             forced_mutable_copy = self.copy(weighted=weighted, data_structure=data_structure, sparse=sparse, immutable=False)
             fresh_copy = forced_mutable_copy.copy(weighted=weighted, data_structure=data_structure, sparse=sparse, immutable=desired_immutable)
-            fresh_copy.hash_labels=hash_labels
+            fresh_copy._hash_labels = hash_labels
             return fresh_copy
 
         # Which data structure should be used ?
@@ -1273,10 +1320,10 @@ def copy(self, weighted=None, data_structure=None, sparse=None, immutable=None,
                 data_structure = "sparse"
 
         G = self.__class__(self, name=self.name(), pos=copy(self._pos),
-                weighted=weighted,
-                data_structure=data_structure)
+                           weighted=weighted,
+                           data_structure=data_structure)
 
-        attributes_to_copy = ('_assoc', '_embedding', 'hash_labels')
+        attributes_to_copy = ('_assoc', '_embedding', '_hash_labels')
         for attr in attributes_to_copy:
             if hasattr(self, attr):
                 copy_attr = {}

src/sage/graphs/graph.py

@@ -570,6 +570,10 @@ class Graph(GenericGraph):
       immutable graph. Note that ``immutable=True`` is actually a shortcut for
       ``data_structure='static_sparse'``. Set to ``False`` by default.
 
+    - ``hash_labels`` -- boolean (default: ``None``); whether to include edge
+      labels during hashing. This parameter defaults to ``True`` if the graph is
+      weighted. This parameter is ignored if the graph is mutable.
+
     - ``vertex_labels`` -- boolean (default: ``True``); whether to allow any
       object as a vertex (slower), or only the integers `0,...,n-1`, where `n`
       is the number of vertices.
@@ -1266,7 +1270,7 @@ def __init__(self, data=None, pos=None, loops=None, format=None,
             self._backend = ib
             self._immutable = True
 
-        self.hash_labels = hash_labels
+        self._hash_labels = hash_labels
 
     # Formats