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

gh-90669: Let typing.Annotated wrap dataclasses annotations #30997

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

GBeauregard
Copy link
Contributor

@GBeauregard GBeauregard commented Jan 28, 2022

We also remove the support for leading spaces in stringified dataclasses annotations (ref https://bugs.python.org/issue46552 ) and allow nested leading Annotated[] annotations (see the PEP).

@JelleZijlstra JelleZijlstra self-requested a review January 28, 2022 22:19
@GBeauregard
Copy link
Contributor Author

GBeauregard commented Jan 28, 2022

This should be ready now; I think this approach is the least invasive and most maintainable: we strip annotations when we find them and otherwise adjust the regex to ignore them in leading position. The annotation stripping could be factored out to another function if we wanted.

Lib/dataclasses.py Outdated Show resolved Hide resolved
Lib/dataclasses.py Outdated Show resolved Hide resolved
Lib/test/test_dataclasses.py Outdated Show resolved Hide resolved
Lib/test/test_dataclasses.py Outdated Show resolved Hide resolved
@GBeauregard
Copy link
Contributor Author

GBeauregard commented Jan 29, 2022

This code is ready for review, but I'll mention another option if you're interested:

The code currently allows anything with a symbol named Annotated on the regex side of things, as we discussed in the bpo issue. I see a way we can support renaming the Annotated symbols within a single regex as long as all nested Annotated symbols are renamed the same way, and then we would inspect the dict to get names similar to how you do for ClassVar. The downside is it complicates the regex processing part of the code a good bit and the regex becomes a lot more opaque. Here was a prototype I was playing with for context:
^(?P<lead>(?:(?P<frontmod>\w+)\s*\.\s*)?(?P<frontsym>\w+)\s*\[\s*)?(?P=lead)*(?:(?P<backmod>\w+)\s*\.)?\s*(?P<backsym>\w+)

An even better approach is using a new regex to nibble at matches at the front of the string annotation and check one by one if they are Annotated until you reach one that isn't. This should be able to support everything.

This is a lot more complicated so I don't intend to introduce it unless you see this as important.

@GBeauregard
Copy link
Contributor Author

GBeauregard commented Mar 19, 2022

@ericvsmith Because my PR removing the need for passing callable() was merged (#31151), I've removed the __call__ method from this PR.

@AlexWaygood AlexWaygood changed the title bpo-46511: Let typing.Annotated wrap dataclasses annotations gh-90669: Let typing.Annotated wrap dataclasses annotations Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants