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

Backport NamedTuple and TypedDict deprecations from Python 3.13 #240

Merged
merged 7 commits into from
Jun 16, 2023

Conversation

JelleZijlstra
Copy link
Member

No description provided.

"""A simple typed namespace. At runtime it is equivalent to a plain dict.

TypedDict creates a dictionary type that expects all of its
TypedDict creates a dictionary type such that a type checker will expect all
Copy link
Member Author

Choose a reason for hiding this comment

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

I took this opportunity to copy-paste the docstring from CPython.

@JelleZijlstra
Copy link
Member Author

Seems like there's some issue with generic namedtuples on 3.12, I'll fix that in a separate PR.

@JelleZijlstra
Copy link
Member Author

Actually the failure is due to this PR (it's because we now re-implement NamedTuple on 3.12). Fixed it.

src/typing_extensions.py Outdated Show resolved Hide resolved
if __fields is None:
__fields = kwargs.items()
def NamedTuple(__typename, __fields=_sentinel, **kwargs):
"""Typed version of namedtuple.
Copy link
Member

@AlexWaygood AlexWaygood Jun 16, 2023

Choose a reason for hiding this comment

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

If we're adding a docstring the normal way here, we no longer need to monkey-patch it onto the function afterwards:

NamedTuple.__doc__ = typing.NamedTuple.__doc__

If you get rid of the docstring monkey-patching, you'll need to ditch this test, as it'll start to fail:

def test_docstring(self):
self.assertEqual(NamedTuple.__doc__, typing.NamedTuple.__doc__)
self.assertIsInstance(NamedTuple.__doc__, str)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I missed that. I think it's better if we present the same docstring across all versions, so I'll remove the monkey-patching.

Comment on lines 5285 to 5286
def test_docstring(self):
self.assertEqual(NamedTuple.__doc__, typing.NamedTuple.__doc__)
self.assertIsInstance(NamedTuple.__doc__, str)
Copy link
Member

Choose a reason for hiding this comment

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

Tbh this test method feels pretty redundant now; I'd just delete the whole method :p

Copy link
Member Author

Choose a reason for hiding this comment

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

It still tests that we have some docstring :)

Copy link
Member

Choose a reason for hiding this comment

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

I'd hope we'd catch it in manual review if somebody proposed deleting the entire docstring in a PR 😄

When I added this test method, my intent was just to test that the monkeypatching in the backport was working correctly. But I guess we can keep it; it doesn't do any harm :)

The PR LGTM!

@JelleZijlstra JelleZijlstra merged commit bc9ce4f into python:main Jun 16, 2023
@JelleZijlstra JelleZijlstra deleted the deprecate branch June 16, 2023 15:15
@JelleZijlstra JelleZijlstra restored the deprecate branch September 10, 2024 23:39
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

Successfully merging this pull request may close these issues.

2 participants