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

Update the cheatsheet for functions and keyword argument typing #15163

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

franekmagiera
Copy link

@franekmagiera franekmagiera commented May 1, 2023

PEP 692 has been accepted and issue #4441 is fixed - added an entry to the cheatsheet mentioning a new way of annotating **kwargs in a function signature.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and all your work on this! Could you add documentation to the TypedDict page: https://mypy.readthedocs.io/en/stable/typed_dict.html ?

@@ -146,6 +146,19 @@ Functions
reveal_type(kwargs) # Revealed type is "dict[str, str]"
request = make_request(*args, **kwargs)
return self.do_api_query(request)

# For more preise keyword typing, you can use `Unpack` along with a
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# For more preise keyword typing, you can use `Unpack` along with a
# For more precise keyword typing, you can use `Unpack` along with a

@JelleZijlstra
Copy link
Member

I don't think mypy enables PEP 692 by default yet, so maybe we should do that first?

Updated and extended the docs section on mixing required non-required
items in a TypedDict. Introduced the use of Required and NotRequired
type qualifiers.
@franekmagiera
Copy link
Author

franekmagiera commented May 2, 2023

Thanks for the comments. I added the section to the TypedDict docs, also updated the section on mixing required and not required items.

As for enabling Unpack - good point! Let's park the doc update for a while. I will try to go through the PEP 692 and see if its specification is enforced by mypy or if we're still missing something and create a separate PR for that. I'm going mountain hiking for a few days, will try to pick that up when I come back.

Comment on lines +253 to +277
As a rule of thumb, if more keys of a particular ``TypedDict`` are required
than not, construct the new type with the ``total`` parameter set to ``True``
and qualify the non required keys using ``NotRequired``. Otherwise, construct
the new type with the ``total`` parameter set to ``False`` and qualify the
required keys with ``Required``.

Using ``Required`` with a total ``TypedDict`` and using ``NotRequired`` with a
partial ``TypedDict`` is redundant. However, it is allowed and can be used to
qualify the required and not required keys explicitly.

If a particular key can accept ``None``, it is recommended to avoid mixing
``Optional`` with either ``Required`` or ``NotRequired`` and use the
``TYPE|None`` notation instead:

.. code-block:: python

# Preferred approach.
class Car(TypedDict):
model: str
owner: NotRequired[str | None]

# Not recommended.
class Car(TypedDict):
model: str
owner: NotRequired[Optional[str]]
Copy link
Author

Choose a reason for hiding this comment

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

Let me know if this is too long. Maybe we could just link to the "How to teach this" section of the PEP 655?

@franekmagiera
Copy link
Author

Hey, sorry for no updates on this (the weather was too nice not to go outside). I started working on enabling the Unpack feature by default, just want to make sure that all the examples from PEP 692 are in the test suite and are passing. Hope to find some time to work on this over the weekend.

@franekmagiera
Copy link
Author

franekmagiera commented Jul 6, 2023

I've opened another PR #15612 that's adding tests from PEP 692, still have trouble with:

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.

None yet

3 participants