Repository: dask/dask
Pull request #11463 (2024-10-29 11:22:22)

Fix `Task.__repr__()` of unpickled object
https://github.com/dask/dask/pull/11378 added a `Task._repr` object that is defined during `Task.__init__()`. In `Task.__repr__()` the object is tested but it doesn't account for the situation where `Task` may have been deserialized and thus `Task.__init__()` is never called, causing that to fail as the attribute was never defined. One other way to fix this would have been to define a default value when the attribute is defined in the scope of the class, but that unfortunately doesn't work due to the `__slots__ = tuple(__annotations__)` in the class. Another alternative to the `hasattr` check could be to define a `@property` with a different name and make `__repr__()` simply return that, which would handle checks regardless of `__init__()` being called, but I think that's too much bloat for little benefit.

- [ ] Tests added / passed
- [ ] Passes `pre-commit run --all-files`


pentschev: @fjetter @phofl would you mind having a look since you're author and approver of the PR that introduced this behavior?
github-actions[bot]: ## Unit Test Results
_See [test report](https://dask.github.io/dask/test_report.html) for an extended history of previous test failures. This is useful for diagnosing flaky tests._

     13 files  ± 0       13 suites  ±0   3h 57m 38s ⏱️ + 16m 6s
 13 241 tests + 1   12 179 ✅ + 1   1 062 💤 ±0  0 ❌ ±0 
138 140 runs  +13  119 051 ✅ +14  19 089 💤  - 1  0 ❌ ±0 

Results for commit 85b1c0dd. ± Comparison against base commit a0783a8d.

[test-results]:data:application/gzip;base64,H4sIAGjOIGcC/03PywqDMBCF4VeRrLuYibn2ZYrmAqFeSkxW0ndvrBpd/t/AgVmJD4NbyLPB9tGQJYd0lc2xS2GetmaUq0LlmvY7ZXj2a8nGbEhR6gvf4bMhCFrNd2EoBhVcjHM8JOZpn1bIzq7TqIFjxWNag9Kn3ab/fV828ziGVIIo3qMBa8ECOGC8/KGdlKZ1XngltKUSe9UK8v0BYcWqKxoBAAA=


Patch:
diff --git a/dask/_task_spec.py b/dask/_task_spec.py
index e02e6cbb1..a319ac865 100644
--- a/dask/_task_spec.py
+++ b/dask/_task_spec.py
@@ -653,7 +653,9 @@ class Task(GraphNode):
         )
 
     def __repr__(self) -> str:
-        if not self._repr:
+        # When `Task` is deserialized the constructor will not run and
+        # `self._repr` is thus undefined.
+        if not hasattr(self, "_repr") or not self._repr:
             self.unpack()
             head = funcname(self.func)
             tail = ")"


Test patch:
diff --git a/dask/tests/test_task_spec.py b/dask/tests/test_task_spec.py
index 0604d95aa..05f92d0cf 100644
--- a/dask/tests/test_task_spec.py
+++ b/dask/tests/test_task_spec.py
@@ -84,6 +84,32 @@ def test_repr():
     assert repr(t) == "<Task 'kwarg' use_kwargs('foo', kwarg='kwarg_value')>"
 
 
+def test_unpickled_repr():
+    t = pickle.loads(pickle.dumps(Task("key", func, "a", "b")))
+    assert repr(t) == "<Task 'key' func('a', 'b')>"
+
+    t = pickle.loads(pickle.dumps(Task("nested", func2, t, t.ref())))
+    assert (
+        repr(t) == "<Task 'nested' func2(<Task 'key' func('a', 'b')>, Alias(key->key))>"
+    )
+
+    def long_function_name_longer_even_longer(a, b):
+        return a + b
+
+    t = pickle.loads(
+        pickle.dumps(Task("long", long_function_name_longer_even_longer, t, t.ref()))
+    )
+    assert repr(t) == "<Task 'long' long_function_name_longer_even_longer(...)>"
+
+    def use_kwargs(a, kwarg=None):
+        return a + kwarg
+
+    t = pickle.loads(
+        pickle.dumps(Task("kwarg", use_kwargs, "foo", kwarg="kwarg_value"))
+    )
+    assert repr(t) == "<Task 'kwarg' use_kwargs('foo', kwarg='kwarg_value')>"
+
+
 def _assert_dsk_conversion(new_dsk):
     vals = [(k, v.key) for k, v in new_dsk.items()]
     assert all(v[0] == v[1] for v in vals), vals