FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add backport of `evaluate_forward_ref` by Daraan · Pull Request #497 · python/typing_extensions · GitHub

  1. python
  2. typing_extensions

Repository navigation

Add backport of evaluate_forward_ref - #497

Merged
JelleZijlstra merged 41 commits into
python:mainfrom
Daraan:3.14/evaluate_forward_ref
Jan 17, 2025
ConversationCommits (41)ChecksFiles changed
Merged

Add backport of evaluate_forward_ref#497
JelleZijlstra merged 41 commits into
python:mainfrom
Daraan:3.14/evaluate_forward_ref

Conversation

Daraan commented Oct 30, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Backports evaluate_forward_ref from JelleZijlstra's PR python/cpython#119891 / PEP 749.

The most tricky part seems to be in recursive ForwardRefs, at least in my tests they worked better in 3.9 and but not in 3.8 and 3.10. If someone has some nice examples how to break it I gladly add them.
Limitation has been added to the docs.

Tests are also tested against cpython main python/cpython@079875e39

AlexWaygood changed the title Add backport of evalaute_forward_ref Add backport of evaluate_forward_ref Oct 30, 2024
Daraan commented Oct 30, 2024
View reviewed changes
Comment thread src/typing_extensions.py
Comment on lines 3826 to +3827
SOURCE = 3
STRING = 3

Daraan Oct 30, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I guess, SOURCE will be removed soon?

JelleZijlstra Nov 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We should just remove SOURCE since we never released this enum.

Daraan Nov 26, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I wonder if the name change & doc changes should possible addressed by another PR

Daraan commented Oct 30, 2024
View reviewed changes
Comment thread src/test_typing_extensions.py Outdated
Comment thread src/test_typing_extensions.py Outdated
Daraan marked this pull request as ready for review October 30, 2024 18:51
JelleZijlstra reviewed Nov 26, 2024
View reviewed changes
Comment thread src/typing_extensions.py
Comment on lines 3826 to +3827
SOURCE = 3
STRING = 3

JelleZijlstra Nov 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We should just remove SOURCE since we never released this enum.

Comment thread src/typing_extensions.py
Comment thread src/typing_extensions.py Outdated
Comment thread src/test_typing_extensions.py Outdated
Comment thread src/test_typing_extensions.py Outdated
Comment thread src/test_typing_extensions.py Outdated
Comment thread src/test_typing_extensions.py Outdated
localns,
type_params,
owner,
) in unroll_cases(cases, locals()):

JelleZijlstra Nov 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not sure all this indirection makes the test cases better. It becomes harder to verify the tests behave as expected with so much logic between the definition of the tests and the actual tests.

Daraan Nov 27, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I share that sentiment. I did some reordering to bring the actual test and case definition closer together, but it does not reduce the overall code in that function.

So far I do not see a good way how to simplify it in a nice way. If you/anyone has suggestions on how to restructure / parts to remove I am open for it.

JelleZijlstra Dec 13, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd simply write explicit test cases for some important cases, instead of attempting to create an abstraction around them.

Daraan Dec 14, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Done, Yes I think that looks better now. Thank you.

Daraan marked this pull request as draft December 14, 2024 13:05
Daraan marked this pull request as ready for review December 14, 2024 15:01
JelleZijlstra merged commit 8184ac6 into python:main Jan 17, 2025
Daraan deleted the 3.14/evaluate_forward_ref branch January 17, 2025 07:39
Daraan mentioned this pull request Apr 1, 2025
Closed
yashwanthatla mentioned this pull request Jun 21, 2025
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JelleZijlstra JelleZijlstra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add typing_extensions.evaluate_forward_ref

2 participants

Back | FazBrowse Home | New Git URL