cattrs icon indicating copy to clipboard operation
cattrs copied to clipboard

NewType support broken by recent `functools.singledispatch` changes

Open bkurtz opened this issue 1 year ago • 3 comments

  • cattrs version: main (290d162a589acf10ea63b825b7b283e23ca7698a)
  • Python version: 3.9.10
  • Operating System: macOS 12.5.1

Description

NewType support was recently added in #255 (see also #94), but appears to have been broken by recent changes to functools.singledispatch that landed in recent pythons (see, e.g. #206, maybe #216).

Aspirationally, something like this should work:

IsoDate = NewType("IsoDate", dt.datetime)
conv.register_structure_hook(IsoDate, ...)

What I Did

python 3.9.10 venv:

Python 3.9.10 (main, Jan 15 2022, 11:48:15) 
[Clang 12.0.0 (clang-1200.0.32.29)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime as dt
>>> from typing import NewType
>>> import cattrs
>>> conv = cattrs.Converter()
>>> IsoDate = NewType("IsoDate", dt.datetime)
>>> conv.register_structure_hook(IsoDate, lambda s, _: IsoDate(dt.datetime.fromisoformat(s)))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/private/tmp/ctest/lib/python3.9/site-packages/cattrs/converters.py", line 254, in register_structure_hook
    self._structure_func.register_cls_list([(cl, func)])
  File "/private/tmp/ctest/lib/python3.9/site-packages/cattrs/dispatch.py", line 57, in register_cls_list
    self._single_dispatch.register(cls, handler)
  File "/usr/local/Cellar/[email protected]/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/functools.py", line 855, in register
    raise TypeError(
TypeError: Invalid first argument to `register()`. <function NewType.<locals>.new_type at 0x1025a4c10> is not a class.

python 3.8.9 (because that's the other one I had handy) venv:

Python 3.8.9 (default, Mar 30 2022, 13:51:17) 
[Clang 13.1.6 (clang-1316.0.21.2.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime as dt
>>> from typing import NewType
>>> import cattrs
>>> conv = cattrs.Converter()
>>> IsoDate = NewType("IsoDate", dt.datetime)
>>> conv.register_structure_hook(IsoDate, lambda s, _: IsoDate(dt.datetime.fromisoformat(s)))
>>> conv.structure("2022-01-01", IsoDate)
datetime.datetime(2022, 1, 1, 0, 0)

(also tested this in python:3.9.9-alpine docker image and get similar results to 3.8.9.)

Not sure what the best strategy for this in cattrs would be, but going to be doing some working around it for our local project this week, and if it were easy I might be talked into opening a PR here too.

FWIW, the other common case we're having issues with is generics (related to #216).

bkurtz avatar Aug 24 '22 23:08 bkurtz

With the fresh light of morning, I'm realizing that #255 implemented structuring of NewTypes out of the box, and that being able to directly register handlers for NewTypes being broken in newer python is already mentioned in #94. It still feels to me like supporting registering for these types out of the box would be a reasonable thing to do, but seems increasingly likely it might make sense to close this as a duplicate of one of the existing issues.

bkurtz avatar Aug 25 '22 14:08 bkurtz

So before we go any further, can you check on 3.10? According to the docs (https://docs.python.org/3/library/typing.html#newtype) in 3.10+ NewType will produce a class, not a function.

Tinche avatar Aug 25 '22 14:08 Tinche

Good idea! Unfortunately, looks like it's still not helpful:

Python 3.10.4 (main, Mar 29 2022, 13:56:05) [GCC 10.3.1 20211027] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime as dt
>>> from typing import NewType
>>> import cattrs
>>> conv = cattrs.Converter()
>>> IsoDate = NewType("IsoDate", dt.datetime)
>>> conv.register_structure_hook(IsoDate, lambda s, _: IsoDate(dt.datetime.fromisoformat(s)))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.10/site-packages/cattrs/converters.py", line 254, in register_structure_hook
    self._structure_func.register_cls_list([(cl, func)])
  File "/usr/local/lib/python3.10/site-packages/cattrs/dispatch.py", line 57, in register_cls_list
    self._single_dispatch.register(cls, handler)
  File "/usr/local/lib/python3.10/functools.py", line 856, in register
    raise TypeError(
TypeError: Invalid first argument to `register()`. __main__.IsoDate is not a class.
>>> type(IsoDate)
<class 'typing.NewType'>

bkurtz avatar Aug 26 '22 03:08 bkurtz

Alright, looking into this as the last thing before I cut a new release.

I'm actually surprised NewTypes used to work with singledispatch at all, before. The original idea was to use the structure hook for the underlying type (in your case, that would be datetime instead of IsoDate).

When it's impossible to use a singledispatch-based hook (as it can commonly happen in more complex scenarios), you'd be expected to use a predicate hook instead.

import datetime as dt
from typing import NewType

import cattrs

conv = cattrs.Converter()
IsoDate = NewType("IsoDate", dt.datetime)

conv.register_structure_hook_func(
    lambda t: t is IsoDate, lambda v, _: dt.datetime.fromisoformat(v)
)
print(conv.structure("2022-01-01", IsoDate))

However since I think this is an interesting idea, we could special case it in register_structure_hook and register_unstructure_hook. The special case would just translate to a structure hook function, like in my snippet. There's already precedent for Unions.

Tinche avatar Sep 29 '22 16:09 Tinche

Would be great if you gave https://github.com/python-attrs/cattrs/pull/310 a try in the next couple of days, then I can cut a release.

Tinche avatar Sep 29 '22 18:09 Tinche

Sorry, closed by accident.

Tinche avatar Sep 29 '22 18:09 Tinche

Tried out #310 and it seems to work well. Thanks for adding!

bkurtz avatar Oct 01 '22 17:10 bkurtz

Thanks for checking. Closing this now.

Tinche avatar Oct 01 '22 18:10 Tinche