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

NewType support broken by recent functools.singledispatch changes #297

Closed
bkurtz opened this issue Aug 24, 2022 · 8 comments
Closed

NewType support broken by recent functools.singledispatch changes #297

bkurtz opened this issue Aug 24, 2022 · 8 comments

Comments

@bkurtz
Copy link
Contributor

bkurtz commented Aug 24, 2022

  • cattrs version: main (290d162)
  • 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/python@3.9/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
Copy link
Contributor Author

bkurtz commented Aug 25, 2022

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.

@Tinche
Copy link
Member

Tinche commented Aug 25, 2022

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.

@bkurtz
Copy link
Contributor Author

bkurtz commented Aug 26, 2022

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'>

@Tinche
Copy link
Member

Tinche commented Sep 29, 2022

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
Copy link
Member

Tinche commented Sep 29, 2022

Would be great if you gave #310 a try in the next couple of days, then I can cut a release.

@Tinche Tinche closed this as completed Sep 29, 2022
@Tinche Tinche reopened this Sep 29, 2022
@Tinche
Copy link
Member

Tinche commented Sep 29, 2022

Sorry, closed by accident.

@bkurtz
Copy link
Contributor Author

bkurtz commented Oct 1, 2022

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

@Tinche
Copy link
Member

Tinche commented Oct 1, 2022

Thanks for checking. Closing this now.

@Tinche Tinche closed this as completed Oct 1, 2022
mergify bot pushed a commit to aws/jsii that referenced this issue Oct 3, 2022
…3 in /packages/@jsii/python-runtime (#3785)

Updates the requirements on [cattrs](https://github.com/python-attrs/cattrs) to permit the latest version.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/python-attrs/cattrs/blob/main/HISTORY.rst">cattrs's changelog</a>.</em></p>
<blockquote>
<h2>22.2.0 (2022-10-03)</h2>
<ul>
<li><em>Potentially breaking</em>: <code>cattrs.Converter</code> has been renamed to <code>cattrs.BaseConverter</code>, and <code>cattrs.GenConverter</code> to <code>cattrs.Converter</code>.
The <code>GenConverter</code> name is still available for backwards compatibility, but is deprecated.
If you were depending on functionality specific to the old <code>Converter</code>, change your import to <code>from cattrs import BaseConverter</code>.</li>
<li><code>NewTypes &lt;https://docs.python.org/3/library/typing.html#newtype&gt;</code>_ are now supported by the <code>cattrs.Converter</code>.
(<code>[#255](python-attrs/cattrs#255) &lt;https://github.com/python-attrs/cattrs/pull/255&gt;</code><em>, <code>[#94](python-attrs/cattrs#94) &lt;https://github.com/python-attrs/cattrs/issues/94&gt;</code></em>, <code>[#297](python-attrs/cattrs#297) &lt;https://github.com/python-attrs/cattrs/issues/297&gt;</code>_)</li>
<li><code>cattrs.Converter</code> and <code>cattrs.BaseConverter</code> can now copy themselves using the <code>copy</code> method.
(<code>[#284](python-attrs/cattrs#284) &lt;https://github.com/python-attrs/cattrs/pull/284&gt;</code>_)</li>
<li>Python 3.11 support.</li>
<li>cattrs now supports un/structuring <code>kw_only</code> fields on attrs classes into/from dictionaries.
(<code>[#247](python-attrs/cattrs#247) &lt;https://github.com/python-attrs/cattrs/pull/247&gt;</code>_)</li>
<li>PyPy support (and tests, using a minimal Hypothesis profile) restored.
(<code>[#253](python-attrs/cattrs#253) &lt;https://github.com/python-attrs/cattrs/issues/253&gt;</code>_)</li>
<li>Fix propagating the <code>detailed_validation</code> flag to mapping and counter structuring generators.</li>
<li>Fix <code>typing.Set</code> applying too broadly when used with the <code>GenConverter.unstruct_collection_overrides</code> parameter on Python versions below 3.9. Switch to <code>typing.AbstractSet</code> on those versions to restore the old behavior.
(<code>[#264](python-attrs/cattrs#264) &lt;https://github.com/python-attrs/cattrs/issues/264&gt;</code>_)</li>
<li>Uncap the required Python version, to avoid problems detailed in <a href="https://iscinumpy.dev/post/bound-version-constraints/#pinning-the-python-version-is-special">https://iscinumpy.dev/post/bound-version-constraints/#pinning-the-python-version-is-special</a>
(<code>[#275](python-attrs/cattrs#275) &lt;https://github.com/python-attrs/cattrs/issues/275&gt;</code>_)</li>
<li>Fix <code>Converter.register_structure_hook_factory</code> and <code>cattrs.gen.make_dict_unstructure_fn</code> type annotations.
(<code>[#281](python-attrs/cattrs#281) &lt;https://github.com/python-attrs/cattrs/issues/281&gt;</code>_)</li>
<li>Expose all error classes in the <code>cattr.errors</code> namespace. Note that it is deprecated, just use <code>cattrs.errors</code>.
(<code>[#252](python-attrs/cattrs#252) &lt;https://github.com/python-attrs/cattrs/issues/252&gt;</code>_)</li>
<li>Fix generating structuring functions for types with quotes in the name.
(<code>[#291](python-attrs/cattrs#291) &lt;https://github.com/python-attrs/cattrs/issues/291&gt;</code>_ <code>[#277](python-attrs/cattrs#277) &lt;https://github.com/python-attrs/cattrs/issues/277&gt;</code>_)</li>
<li>Fix usage of notes for the final version of <code>PEP 678 &lt;https://peps.python.org/pep-0678/&gt;</code><em>, supported since <code>exceptiongroup&gt;=1.0.0rc4</code>.
(<code>[#303](python-attrs/cattrs#303) &lt;303 &lt;https://github.com/python-attrs/cattrs/pull/303&gt;</code></em>)</li>
</ul>
<h2>22.1.0 (2022-04-03)</h2>
<ul>
<li>cattrs now uses the CalVer versioning convention.</li>
<li>cattrs now has a detailed validation mode, which is enabled by default. Learn more <code>here &lt;https://cattrs.readthedocs.io/en/latest/validation.html&gt;</code>_.
The old behavior can be restored by creating the converter with <code>detailed_validation=False</code>.</li>
<li><code>attrs</code> and dataclass structuring is now ~25% faster.</li>
<li>Fix an issue structuring bare <code>typing.List</code> s on Pythons lower than 3.9.
(<code>[#209](python-attrs/cattrs#209) &lt;https://github.com/python-attrs/cattrs/issues/209&gt;</code>_)</li>
<li>Fix structuring of non-parametrized containers like <code>list/dict/...</code> on Pythons lower than 3.9.
(<code>[#218](python-attrs/cattrs#218) &lt;https://github.com/python-attrs/cattrs/issues/218&gt;</code>_)</li>
<li>Fix structuring bare <code>typing.Tuple</code> on Pythons lower than 3.9.
(<code>[#218](python-attrs/cattrs#218) &lt;https://github.com/python-attrs/cattrs/issues/218&gt;</code>_)</li>
<li>Fix a wrong <code>AttributeError</code> of an missing <code>__parameters__</code> attribute. This could happen
when inheriting certain generic classes – for example <code>typing.*</code> classes are affected.
(<code>[#217](python-attrs/cattrs#217) &lt;https://github.com/python-attrs/cattrs/issues/217&gt;</code>_)</li>
<li>Fix structuring of <code>enum.Enum</code> instances in <code>typing.Literal</code> types.
(<code>[#231](python-attrs/cattrs#231) &lt;https://github.com/python-attrs/cattrs/pull/231&gt;</code>_)</li>
<li>Fix unstructuring all tuples - unannotated, variable-length, homogenous and heterogenous - to <code>list</code>.
(<code>[#226](python-attrs/cattrs#226) &lt;https://github.com/python-attrs/cattrs/issues/226&gt;</code>_)</li>
<li>For <code>forbid_extra_keys</code> raise custom <code>ForbiddenExtraKeyError</code> instead of generic <code>Exception</code>.
(<code>[#225](python-attrs/cattrs#225) &lt;https://github.com/python-attrs/cattrs/pull/225&gt;</code>_)</li>
<li>All preconf converters now support <code>loads</code> and <code>dumps</code> directly. See an example <code>here &lt;https://cattrs.readthedocs.io/en/latest/preconf.html&gt;</code>_.</li>
</ul>

</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/python-attrs/cattrs/commit/405f0291b958ae9eb45ee38febeb91fb65dd644f"><code>405f029</code></a> v22.2.0</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/89de04f57aa774d6abfb0ae62517dc8a8064e3c2"><code>89de04f</code></a> Fix some mor</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/0abbf271461babca203862f3581c20743f6118e0"><code>0abbf27</code></a> Fix tests</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/3b750439aec826a8fd20976ca113f30a27e75408"><code>3b75043</code></a> <strong>notes</strong> is list[str]</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/906b95cfef4903e2d6247abf0fdd72f7da21617a"><code>906b95c</code></a> Reorder HISTORY</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/ed7f86a0ccd9adeab895b9afac2dea69fa02bcff"><code>ed7f86a</code></a> Improve NewTypes (<a href="https://github-redirect.dependabot.com/python-attrs/cattrs/issues/310">#310</a>)</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/e7926599ad44e07d8325ae4072626e1a24705542"><code>e792659</code></a> Fix missing imports of preconf converters (<a href="https://github-redirect.dependabot.com/python-attrs/cattrs/issues/309">#309</a>)</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/e425d6378aa8dfc74bbdd9e152365e1b962fd8cf"><code>e425d63</code></a> Reorder HISTORY</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/cc56b2b873852a4e91b16706a39da00755c87759"><code>cc56b2b</code></a> Remove spurious type comment</li>
<li><a href="https://github.com/python-attrs/cattrs/commit/cbd6f29d2c0ebc40805b3ca0d81accfc330eb10c"><code>cbd6f29</code></a> Reformat</li>
<li>Additional commits viewable in <a href="https://github.com/python-attrs/cattrs/compare/v1.8.0...v22.2.0">compare view</a></li>
</ul>
</details>
<br />


Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants