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

bpo-46066: Deprecate kwargs syntax for TypedDict definitions by 97littleleaf11 · Pull Request #31126 · python/cpython · GitHub

/ cpython Public

bpo-46066: Deprecate kwargs syntax for TypedDict definitions - #31126

Merged
JelleZijlstra merged 11 commits into
python:mainfrom
97littleleaf11:bpo-46066
Feb 17, 2022
Merged

bpo-46066: Deprecate kwargs syntax for TypedDict definitions#31126
JelleZijlstra merged 11 commits into
python:mainfrom
97littleleaf11:bpo-46066

Conversation

97littleleaf11 commented Feb 4, 2022
edited
Loading

Copy link
Copy Markdown
Contributor

This comment was marked as resolved.

AlexWaygood commented Feb 4, 2022
edited
Loading

Copy link
Copy Markdown
Member

Surely this is a breaking change — even if type-checkers only understand the literal-dict assignment-based syntax, I'm not sure we can change the runtime behaviour like this without a deprecation period.

Copy link
Copy Markdown
Contributor Author

Yeah, it changes the typing module. However I seldom see this syntax in real-world projects since we already have class based one.

Copy link
Copy Markdown
Contributor Author

I am not really familiar with the cpython developing period. It's not a big deal since neither mypy nor other widely-used type checkers implement this feature and I barely see this syntax in real world projects.

Copy link
Copy Markdown
Member

Yeah, it changes the typing module. However I seldom see this syntax in real-world projects since we already have class based one.

I appreciate that — but given that this is a change to the stdlib, I think the standard deprecation policy probably still applies :)

Copy link
Copy Markdown
Contributor Author

I think the standard deprecation policy probably still applies :)

Thanks for pointing out!

Copy link
Copy Markdown
Member

For now, I think I would:

  • Change the docs to state that this is deprecated in 3.11, will be removed in 3.13, and probably won't be understood by type checkers like mypy.
  • Change the runtime behaviour to emit a warning if a user tries to construct a TypedDict with the kwargs-based syntax.

Here's an example of a previous PR deprecating features, that you could look to as an example: #23064

97littleleaf11 marked this pull request as ready for review February 7, 2022 17:59
Comment thread Lib/typing.py
Comment thread Doc/library/typing.rst
AlexWaygood self-requested a review February 7, 2022 21:17
Comment thread Doc/library/typing.rst Outdated
Comment thread Doc/library/typing.rst Outdated
Comment thread Doc/library/typing.rst Outdated
Comment thread Lib/test/test_typing.py Outdated
Comment thread Lib/typing.py Outdated
97littleleaf11 and others added 2 commits February 8, 2022 23:46
Co-authored-by: AlexWaygood <Alex.Waygood@Gmail.com>
97littleleaf11 changed the title bpo-46066: Remove kwargs syntax for TypedDict definition bpo-46066: Deprecate kwargs syntax for TypedDict definitions Feb 8, 2022
Comment thread Misc/ACKS Outdated
Ka-Ping Yee
Chi Hsuan Yen
Jason Yeo
Jingchen Ye

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

It feels absurd to bring this up (sorry!!), but: I think "Jingchen Ye" should go before "Ka-Ping Yee", if we're keeping this list alphabetised.

Copy link
Copy Markdown
Member

Also, it looks like there's some trailing whitespace in some of the documentation changes you've made, which is making the documentation-related tests fail -- could you possibly fix those issues?

Copy link
Copy Markdown
Contributor Author

@AlexWaygood Thanks for your reviews! btw, it seems that CI reports wrong line number about the trailing whitespace.

Copy link
Copy Markdown
Member

@AlexWaygood Thanks for your reviews!

No worries, it's a good PR!

btw, it seems that CI reports wrong line number about the trailing whitespace.

Huh — no idea what might be causing that :)

AlexWaygood left a comment

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

LGTM

Copy link
Copy Markdown
Member

As this is a deprecation of an existing feature, I think it probably warrants a mention in "What's New in Python 3.11". I think @JelleZijlstra is planning on doing a PR for that document mentioning a bunch of recent changes to typing, though, so perhaps he'll be kind enough to include a mention of this as well :)

Copy link
Copy Markdown
Member

@gvanrossum, could we possibly get the full test suite run on this PR, please? :)

gvanrossum left a comment

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 can't stand "firstly" and "secondly" for some reason. :-(

Copy link
Copy Markdown
Contributor Author

I can't stand "firstly" and "secondly" for some reason. :-(

How about "The first one" and "The other one"?

Copy link
Copy Markdown
Member

How about "The first one" and "The other one"?

Probably, a bullet list will be better. Like this:

[...] syntactic forms:

  • using a literal dict as the second argument:

    Point2D = TypedDict('Point2D', {'x': int, 'y': int, 'label': str})
    
  • using keyword arguments:

    Point2D = TypedDict('Point2D', x=int, y=int, label=str)
    

JelleZijlstra left a comment

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

Here's a concrete wording suggestion that avoids "Firstly" and "Secondly"

Comment thread Doc/library/typing.rst Outdated
Comment on lines +1391 to +1393
support :pep:`526`, ``TypedDict`` supports two additional equivalent
syntactic forms::
syntactic forms. Firstly, using a literal :class:`dict` as the
second argument::

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 suggest just writing "a TypedDict may be created using a functional form". This parallels https://docs.python.org/3.10/library/enum.html#functional-api

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 this!

Comment thread Doc/library/typing.rst Outdated
Point2D = TypedDict('Point2D', x=int, y=int, label=str)
Point2D = TypedDict('Point2D', {'x': int, 'y': int, 'label': str})

Secondly, using keyword arguments::

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
Suggested change
Secondly, using keyword arguments::
Keyword arguments may also be used::

Copy link
Copy Markdown
Member

I was just hoping to see “first“ and “second”. No “ly” needed, these are flat adverbs.

Copy link
Copy Markdown
Member

In a few days Jelle can likely merge this himself. :-)

Comment thread Doc/library/typing.rst Outdated

Copy link
Copy Markdown
Member

Congrats Jelle!

Copy link
Copy Markdown
Member

And congrats @97littleleaf11 on your first CPython contribution!

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.

TypedDict alternative definition syntax with keyword args is confusing

7 participants


Back | FazBrowse Home | New Git URL