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

Add note instead of catching and raising unhashable exception #734

Merged
merged 4 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pydra/engine/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -999,9 +999,9 @@ def get_value(
"that don't return stable hash values for specific object "
"types across multiple processes (see bytes_repr() "
'"singledispatch "function in pydra/utils/hash.py).'
"You may need to implement a specific `bytes_repr()` "
'"singledispatch overload"s or `__bytes_repr__()` '
"dunder methods to handle one or more types in "
"You may need to write specific `bytes_repr()` "
"implementations (see `pydra.utils.hash.register_serializer`) or a "
"`__bytes_repr__()` dunder methods to handle one or more types in "
"your interface inputs."
)
_, split_depth = TypeParser.strip_splits(self.type)
Expand Down
8 changes: 4 additions & 4 deletions pydra/engine/submitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@ async def expand_workflow(self, wf, rerun=False):
"that don't return stable hash values for specific object "
"types across multiple processes (see bytes_repr() "
'"singledispatch "function in pydra/utils/hash.py).'
"You may need to implement a specific `bytes_repr()` "
'"singledispatch overload"s or `__bytes_repr__()` '
"dunder methods to handle one or more types in "
"your interface inputs."
"You may need to write specific `bytes_repr()` "
"implementations (see `pydra.utils.hash.register_serializer`) "
"or `__bytes_repr__()` dunder methods to handle one "
"or more types in your interface inputs."
)
raise RuntimeError(msg)
for task in tasks:
Expand Down
12 changes: 1 addition & 11 deletions pydra/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,11 +1 @@
from pathlib import Path
import platformdirs
from pydra._version import __version__

user_cache_dir = Path(
platformdirs.user_cache_dir(
appname="pydra",
appauthor="nipype",
version=__version__,
)
)
from .misc import user_cache_dir, add_exc_note # noqa: F401
18 changes: 12 additions & 6 deletions pydra/utils/hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from filelock import SoftFileLock
import attrs.exceptions
from fileformats.core import FileSet
from . import user_cache_dir
from . import user_cache_dir, add_exc_note

logger = logging.getLogger("pydra")

Expand Down Expand Up @@ -197,10 +197,6 @@ def __contains__(self, object_id):
return object_id in self._hashes


class UnhashableError(ValueError):
"""Error for objects that cannot be hashed"""


def hash_function(obj, **kwargs):
"""Generate hash of object."""
return hash_object(obj, **kwargs).hex()
Expand All @@ -224,7 +220,17 @@ def hash_object(
try:
return hash_single(obj, cache)
except Exception as e:
raise UnhashableError(f"Cannot hash object {obj!r} due to '{e}'") from e
tp = type(obj)
add_exc_note(
e,
(
f"and therefore cannot hash `{obj!r}` of type "
f"`{tp.__module__}.{tp.__name__}`. Consider implementing a "
"specific `bytes_repr()`(see pydra.utils.hash.register_serializer) "
"or a `__bytes_repr__()` dunder methods for this type"
),
)
raise e


def hash_single(obj: object, cache: Cache) -> Hash:
Expand Down
33 changes: 33 additions & 0 deletions pydra/utils/misc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from pathlib import Path
import platformdirs
from pydra._version import __version__

user_cache_dir = Path(
platformdirs.user_cache_dir(
appname="pydra",
appauthor="nipype",
version=__version__,
)
)


def add_exc_note(e: Exception, note: str) -> Exception:
"""Adds a note to an exception in a Python <3.11 compatible way

Parameters
----------
e : Exception
the exception to add the note to
note : str
the note to add

Returns
-------
Exception
returns the exception again
"""
if hasattr(e, "add_note"):
e.add_note(note)
else:
e.args = (e.args[0] + "\n" + note,)

Check warning on line 32 in pydra/utils/misc.py

View check run for this annotation

Codecov / codecov/patch

pydra/utils/misc.py#L32

Added line #L32 was not covered by tests
return e
26 changes: 25 additions & 1 deletion pydra/utils/tests/test_hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from fileformats.text import TextFile
from ..hash import (
Cache,
UnhashableError,
bytes_repr,
hash_object,
register_serializer,
Expand Down Expand Up @@ -386,3 +385,28 @@ def test_persistent_hash_cache_not_dir(text_file):
"""
with pytest.raises(ValueError, match="is not a directory"):
PersistentCache(text_file.fspath)


def test_unhashable():
"""
Test that an error is raised if an unhashable object is provided
"""

class A:

def __bytes_repr__(self, cache: Cache) -> ty.Generator[bytes, None, None]:
raise TypeError("unhashable")

def __repr__(self):
return "A()"

# hash_object(A())

with pytest.raises(
TypeError,
match=(
"unhashable\nand therefore cannot hash `A\(\)` of type "
"`pydra.utils.tests.test_hash.A`"
),
):
hash_object(A())
Loading