python-attrs / cattrs

Composable custom class converters for attrs, dataclasses and friends.
https://catt.rs
MIT License
818 stars 113 forks source link

Converter copy does not copy unstructure hooks #398

Closed danielnelson closed 1 year ago

danielnelson commented 1 year ago

Description

I wanted to create a base Converter with some common unstructuring hooks, then copy it in other places and add extra hooks. After copying the Converter I found that the unstructuring hooks from the source Converter no longer work.

What I Did

from dataclasses import dataclass
from functools import partial
from typing import Any

import cattrs

@dataclass
class C:
    value: int

@dataclass
class D:
    child: Any

# https://github.com/python-attrs/cattrs/issues/320
def _unstructure_any(converter, value):
    return converter.unstructure(value, unstructure_as=value.__class__)

converter = cattrs.Converter()
converter.register_unstructure_hook_func(
    lambda t: t is Any, partial(_unstructure_any, converter)
)
d = D(child=C(value=42))
actual = converter.unstructure(d)
expected = {"child": {"value": 42}}
assert expected == actual

converter = converter.copy()
d = D(child=C(value=42))
actual = converter.unstructure(d)
expected = {"child": {"value": 42}}
assert expected == actual, "failure"
Traceback (most recent call last):
  File "tmp.py", line 36, in <module>
    assert expected == actual, "failure"
AssertionError: failure
danielnelson commented 1 year ago

I think the head of the handler pairs needs to be copied, because we prepend new hooks to the front, but we are currently copying the tail. Maybe a change along these lines:

diff --git a/src/cattrs/dispatch.py b/src/cattrs/dispatch.py
index f8a5445..c6542ea 100644
--- a/src/cattrs/dispatch.py
+++ b/src/cattrs/dispatch.py
@@ -137,4 +137,4 @@ class FunctionDispatch:
         return len(self._handler_pairs)

     def copy_to(self, other: "FunctionDispatch", skip: int = 0):
-        other._handler_pairs.extend(self._handler_pairs[skip:])
+        other._handler_pairs = self._handler_pairs[:-skip] + other._handler_pairs
Tinche commented 1 year ago

Yeah you're right. The tests only test copying of singledispatch hooks, oops. Looking into it at https://github.com/python-attrs/cattrs/pull/399

Tinche commented 1 year ago

Should be fixed on main, let me know!