-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
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-82129: Provide __annotate__
method for dataclasses from make_dataclass
#122262
base: main
Are you sure you want to change the base?
Conversation
…ake_dataclass`
Test failure:
is expected, I just don't want to touch tests at this point. |
__annotate__
method for dataclasses from make_dataclass
__annotate__
method for dataclasses from make_dataclass
I mentioned it on the issue but with this change
If you want to avoid the typing import this way you need to put some placeholder for This doesn't happen on the 3.13 version proposed because dataclasses doesn't evaluate the strings so the import doesn't get called by dataclasses when it looks at the annotations. |
You might be able to get around @DavidCEllis's point by adding some internal-only flag to tell |
Perhaps you can take advantage of If it's called with |
Yes, this is my go-to idea right now :) |
This PR still does not account for the problem mentioned by @DavidCEllis I will try to work around this problem. |
But, since cc @JelleZijlstra about my question in #122285 |
Now Thanks everyone for your help and ideas! 🤝 |
@@ -1549,11 +1550,29 @@ class C(Base): | |||
seen.add(name) | |||
annotations[name] = tp | |||
|
|||
def annotate_method(format): | |||
typing = sys.modules.get("typing") | |||
if typing is None and format == annotationlib.Format.FORWARDREF: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also avoid importing typing for the SOURCE format here I think; is that worth it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure we can. I need _convert_to_source
, there can be complex annotations that should be formatted properly. I will open a new issue about converting annotations to string with public API though. Right now I don't see a clear way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened #124412 for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me.
Misc/NEWS.d/next/Library/2024-09-18-09-15-40.gh-issue-82129.GQwt3u.rst
Outdated
Show resolved
Hide resolved
…wt3u.rst Co-authored-by: Carl Meyer <[email protected]>
I realized there's now new way to convert annotations to source. Fixing! |
Blocked by #125507 |
This is my plan for python3.14+
While #122232 can be backported to older versions and solve their problems, this one is actually a correct way to solve this problem for the future.
This is a WIP, because I think that we should first decide on #122232