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

gh-137840: Implement PEP 728 (closed and extra_items in typing.TypedDict) by angela-tarantula · Pull Request #137933 · python/cpython · GitHub

Repository navigation

gh-137840: Implement PEP 728 (closed and extra_items in typing.TypedDict) - #137933

Merged
JelleZijlstra merged 20 commits into
python:mainfrom
angela-tarantula:fix-issue-137840
Oct 3, 2025
Merged

JelleZijlstra merged 20 commits into
python:mainfrom
angela-tarantula:fix-issue-137840

Conversation

angela-tarantula commented Aug 19, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

Features:

  1. New class parameters on TypedDict: closed and extra_items.
  2. Raise TypeError when both parameters are specified together.
  3. New introspection attributes __closed__ and __extra_items__.
  4. typing.NoExtraItems is now a C-level immortal singleton, co-located with NoDefault in typevarobject.c.
  5. Tests covering PEP 728 runtime behavior.
  6. Suggested new docstring for TypedDict.

Not included in this PR:

Comment thread Lib/test/test_typing.py
TypeError,
"Cannot combine closed=True and extra_items"
):
class TD(TypedDict, closed=True, extra_items=range):

Copy link
Copy Markdown
Contributor

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

Is range the builtin here?
That's a bit strange.

angela-tarantula Aug 19, 2025 •
edited
Loading

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 agree it looks a bit odd as an arbitrary extra_items value, but it's valid, since it just means extra keys must have values assignable to range. For context, this test is copied from typing_extensions, where it was used to cover the same case. We could edit it for readability, but I kind of like it as a reminder for maintainers that extra_items accepts any type, not just the obvious ones.

Copy link
Copy Markdown
Contributor

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

That could be valid for a regular test that validates stored values.

this however is a negative test, isn’t it?

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

It's still valid/correct as a negative test. By "valid" do you mean "correct" or "readable?" Passing range into extra_items is valid, and passing closed=True at the same time should raise a runtime error.

dimaqq Aug 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Sorry I was being terse.

I'm trying to say that this test:

        class Child(Base, extra_items=int):
            a: str

        self.assertEqual(Child.__required_keys__, frozenset({'a'}))
        self.assertEqual(Child.__optional_keys__, frozenset({}))
        self.assertEqual(Child.__readonly_keys__, frozenset({}))
        self.assertEqual(Child.__mutable_keys__, frozenset({'a'}))
        self.assertEqual(Child.__annotations__, {"a": str})
        self.assertIs(Child.__extra_items__, int)
        self.assertIsNone(Child.__closed__)

Could have an extra_items=range counterpart.

That would make a solid test, both understandable and useful.

Meanwhile, the negative test, class TD(TypedDict, closed=True, extra_items=range): --> error would be better served with a simpler, more straightforward extra_items=int argument.

Copy link
Copy Markdown
Contributor

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

P.S. my comment overall is minor, please don't let me stop your work!

angela-tarantula Aug 20, 2025 •
edited
Loading

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

Thanks for clarifying. I thought about it some more.

I don't think introducing an extra_items=range counterpart would actually widen the test coverage, since it wouldn't be exercising any new behavior. PEP 728 splits responsibilities between runtime and type checker behavior. While extra_items is only supposed to accept a valid type expression, validating that is the type checker's job (e.g. MyPy's valid-type error), not the runtime's. The runtime just stores whatever is passed in.

So the real subject under test is simply:

Child.__extra_items__ must correctly store the value passed into extra_items.

We don't need multiple values to prove that behavior.

And although range is a less obvious type, I think it makes sense in the negative test. That test is specifically asserting the error message “Cannot combine closed=True and extra_items”. Using range highlights that the failure comes from the combination, not from range itself being an invalid value of extra_items.

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 tend to use range sometimes as it's a builtin type that isn't generic (like list) and doesn't participate in promotion weirdness (like float and historically bytes), so it's a good basic type to test with.

Plus, people who forget that range is a type get to learn something :)

Copy link
Copy Markdown
Contributor Author

Ready for review! @PIG208 @JelleZijlstra

Comment thread Objects/typevarobject.c Outdated

PyObject _Py_NoDefaultStruct = _PyObject_HEAD_INIT(&_PyNoDefault_Type);

/* NoExtraItems: a marker object for TypedDict extra_items when it's unset. */

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

Is this implemented in C just for analogy with NoDefault? NoDefault is in C because TypeVar uses it and TypeVar has to be in C because it has native syntax, but TypedDict is all Python code, so I'd rather keep NoExtraItems also just Python.

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

Also if we get PEP 661 implemented in 3.15 (I hope we will), this can and should use it.

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

My bad, I thought we wanted it in C like NoDefault, but yeah since TypedDict is all Python we can keep it like that. I'll reverse course on this. PEP 661 looks interesting! Must being annoying to maintain so many ad-hoc sentinels.

Comment thread Objects/typevarobject.c Outdated
Comment thread Objects/typevarobject.c Outdated
Comment thread Lib/typing.py Outdated
Comment thread Objects/typevarobject.c Outdated

PyObject _Py_NoDefaultStruct = _PyObject_HEAD_INIT(&_PyNoDefault_Type);

/* NoExtraItems: a marker object for TypedDict extra_items when it's unset. */

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

Also if we get PEP 661 implemented in 3.15 (I hope we will), this can and should use it.

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

4 participants


Back | FazBrowse Home | New Git URL