Merge pull request #91 from evo-company/add-unhashable-hints

add more hints for unhashable types

docker-compose.yaml

@@ -11,6 +11,8 @@ services:
     environment:
       PYTHONPATH: .
       PYTHONBUFFERED: 1
+      PYTHONWARNINGS: |
+        ignore::DeprecationWarning
 
   docs:
     <<: *base
@@ -43,7 +45,7 @@ services:
     <<: *test-base
     depends_on:
       - pg
-    entrypoint: py.test
+    entrypoint: py.test -v
 
   pg:
     image: postgres:13-alpine

docs/changelog/changes_07.rst

@@ -32,6 +32,7 @@ Changes in 0.7
   - Added result cache - it is possible now to specify ``@cached`` directive to cache parts of the query. :ref:`Check cache documentation <caching-doc>`
   - ``Link(requires=['a', 'b'])`` can be specified as a list of strings. It is useful when you want to require multiple fields at once. It will pass a list of dicts to the resolver.
   - Added support for Python 3.11
+  - Added hints when failing on unhashable return values
 
 Backward-incompatible changes
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

hiku/engine.py

@@ -346,21 +346,31 @@ def link_ref_many(
 }
 
 
-HASH_HINT = "\nHint: Consider adding __hash__ method or use hashable type."
-DATACLASS_HINT = "\nHint: Use @dataclass(frozen=True) to make object hashable."
+HASH_HINT = "\nHint: Consider adding __hash__ method or use hashable type for '{!r}'.".format  # noqa: E501
+SEQ_HINT = "\nHint: Consider using tuple instead of '{}'.".format
+DATACLASS_FROZEN_HINT = "\nHint: Use @dataclass(frozen=True) on '{}'.".format
+DATACLASS_FIELD_HINT = "\nHint: Field '{}.{}' of type '{}' is not hashable.".format  # noqa: E501
 
 
 def _hashable_hint(obj: Any) -> str:
-    if isinstance(obj, Sequence):
+    if isinstance(obj, (list, set)):
+        return SEQ_HINT(type(obj).__name__)
+
+    if isinstance(obj, Sequence) and not isinstance(obj, str):
         return _hashable_hint(obj[0])
 
-    if (
-        dataclasses.is_dataclass(obj)
-        and not getattr(obj, dataclasses._PARAMS).frozen  # type: ignore[attr-defined]  # noqa: E501
-    ):
-        return DATACLASS_HINT
+    if dataclasses.is_dataclass(obj):
+        if not getattr(obj, dataclasses._PARAMS).frozen:  # type: ignore[attr-defined]  # noqa: E501
+            return DATACLASS_FROZEN_HINT(obj.__class__.__name__)
+
+        for f in dataclasses.fields(obj):
+            val = getattr(obj, f.name)
+            if not _is_hashable(val):
+                return DATACLASS_FIELD_HINT(
+                    obj.__class__.__name__, f.name, type(val).__name__
+                )
 
-    return HASH_HINT
+    return HASH_HINT(obj)
 
 
 def _check_store_links(
@@ -486,7 +496,7 @@ def __init__(
         graph: Graph,
         query: QueryNode,
         ctx: 'Context',
-        cache: CacheInfo = None
+        cache: Optional[CacheInfo] = None
     ) -> None:
         self._queue = queue
         self._task_set = task_set

hiku/graph.py

@@ -77,6 +77,10 @@ class Option(AbstractOption):
 
         Option('size', Integer, default=100)
 
+    Example with TypeRef type(ref can point either to Node or data type)::
+
+        Option('filter', TypeRef['FilterInput'])
+
     """
     def __init__(
         self,
@@ -268,11 +272,11 @@ def get_type_enum(type_: TypingMeta) -> t.Tuple[Const, str]:
 
 RootLinkOne = RootLinkT[LR]
 RootLinkMaybe = RootLinkT[MaybeLink[LR]]
-RootLinkMany = RootLinkT[List[LR]]
+RootLinkMany = RootLinkT[List[LR]]  # type: ignore[type-var]
 
-LinkOne = LinkT[LT, List[LR]]
-LinkMaybe = LinkT[LT, List[MaybeLink[LR]]]
-LinkMany = LinkT[LT, List[List[LR]]]
+LinkOne = LinkT[LT, List[LR]]  # type: ignore[type-var]
+LinkMaybe = LinkT[LT, List[MaybeLink[LR]]]  # type: ignore[type-var]
+LinkMany = LinkT[LT, List[List[LR]]]  # type: ignore[type-var]
 
 LinkFunc = t.Union[
     RootLinkOne,

hiku/validate/query.py

@@ -226,7 +226,8 @@ def visit_record(self, type_: RecordMeta) -> None:
         return None
 
     def visit_typeref(self, type_: TypeRefMeta) -> None:
-        assert type_.__type_name__ in self._data_types, type_.__type_name__
+        assert type_.__type_name__ in self._data_types, \
+            f'"{type_.__type_name__}" type is not present in graph data_types'
         self.visit(self._data_types[type_.__type_name__])
 
 

lets.yaml

@@ -52,7 +52,7 @@ commands:
   test:
     description: Run tests
     depends: [_build-tests]
-    cmd: docker-compose run --rm pytest
+    cmd: [docker-compose, run, --rm, pytest]
 
   bench:
     description: Run benchmarks

tests/test_hashable.py

@@ -1,5 +1,6 @@
 import pytest
 
+from typing import List
 from dataclasses import dataclass
 
 from hiku.builder import build, Q
@@ -62,7 +63,7 @@ def map_field(f, user):
             Link('info', TypeRef['userInfo'], direct_link, requires='_data'),
         ]),
         Root([
-            Link('user', TypeRef['user'],  link_user,  requires=None),
+            Link('user', TypeRef['user'], link_user, requires=None),
         ]),
     ])
 
@@ -112,7 +113,7 @@ def link_tags(ids):
             Link('tags', Sequence[TypeRef['tags']], link_tags, requires='id'),
         ]),
         Root([
-            Link('user', TypeRef['user'],  link_user,  requires=None),
+            Link('user', TypeRef['user'], link_user, requires=None),
         ]),
     ])
 
@@ -151,7 +152,7 @@ def map_field(f, user):
             Field('id', Integer, user_fields),
         ]),
         Root([
-            Link('user', TypeRef['user'],  link_user,  requires=None),
+            Link('user', TypeRef['user'], link_user, requires=None),
         ]),
     ])
 
@@ -182,8 +183,8 @@ def user_fields(fields, ids):
             Field('id', Integer, user_fields),
         ]),
         Root([
-            Field('_user', Any,  user_data),
-            Link('user', TypeRef['user'],  direct_link,  requires='_user'),
+            Field('_user', Any, user_data),
+            Link('user', TypeRef['user'], direct_link, requires='_user'),
         ]),
     ])
 
@@ -229,3 +230,100 @@ def link_options(opts):
         r"Can't store link values, node: '__root__', link: 'user', "
         r"expected: hashable object, returned:(.*User)"
     )
+
+
+@pytest.mark.parametrize('typ, expected', [
+    (list, 'list'),
+    (set, 'set'),
+])
+def test_hint_unhashble_type(typ, expected):
+    GRAPH = Graph([
+        Node('user', [
+            Field('tags', Sequence[String], lambda fields, ids: None),
+        ]),
+        Root([
+            Link(
+                'user',
+                TypeRef['user'],
+                lambda: typ(['tag1']),
+                requires=None,
+            ),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([Q.user[Q.tags]]))
+
+    err.match(f"Hint: Consider using tuple instead of '{expected}'.")
+
+
+def test_hint_unhashble_type_in_tuple():
+    GRAPH = Graph([
+        Node('user', [
+            Field('tags', Sequence[String], lambda fields, ids: [[]]),
+        ]),
+        Root([
+            Link(
+                'user',
+                TypeRef['user'],
+                lambda: tuple([{}, {}]),
+                requires=None,
+            ),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([Q.user[Q.tags]]))
+
+    err.match(
+        "Hint: Consider adding __hash__ method or use hashable type for '{}'.")
+
+
+def test_hint_frozen_dataclass():
+    @dataclass
+    class User:
+        tags: List[str]
+
+    GRAPH = Graph([
+        Node('user', [
+            Field('tags', Sequence[String], lambda fields, ids: None),
+        ]),
+        Root([
+            Link(
+                'user',
+                TypeRef['user'],
+                lambda: User(['tag1']),
+                requires=None,
+            ),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([Q.user[Q.tags]]))
+
+    err.match("Hint: Use @dataclass\\(frozen=True\\) on 'User'.")
+
+
+def test_hint_dataclass_unhashable_field():
+    @dataclass(frozen=True)
+    class User:
+        tags: List[str]
+
+    GRAPH = Graph([
+        Node('user', [
+            Field('tags', Sequence[String], lambda fields, ids: None),
+        ]),
+        Root([
+            Link(
+                'user',
+                TypeRef['user'],
+                lambda: User(['tag1']),
+                requires=None,
+            ),
+        ]),
+    ])
+
+    with pytest.raises(TypeError) as err:
+        execute(GRAPH, build([Q.user[Q.tags]]))
+
+    err.match("Hint: Field 'User.tags' of type 'list' is not hashable.")