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

Apply TypeVar defaults to functions (PEP 696) #15387

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Jun 7, 2023

Use TypeVar defaults to resolve fallback return type of functions.

Note: Defaults for TypeVarTuples don't yet work, probably a result of the limited support for Unpack / TypeVarTuple.

Ref: #14851

@cdce8p
Copy link
Collaborator Author

cdce8p commented Jun 7, 2023

@hauntsaninja Any idea why the primer comment job is failing? The primer run with artifact looks fine
Primer: https://github.com/python/mypy/actions/runs/5198160649
Comment: https://github.com/python/mypy/actions/runs/5198345718/jobs/9374346954

This seems to have started last night. Strangely enough, everything continues to work fine on my fork with the same config.
It seems the listWorkflowRunArtifacts endpoint might be an issue, but calling it manually works fine 🤷🏻‍♂️

--
Update: Seems the issue resolved itself.

@cdce8p cdce8p force-pushed the TypeVar-03-apply-functions branch from 20f2c17 to 85eb940 Compare June 7, 2023 14:54
@github-actions

This comment has been minimized.

@cdce8p
Copy link
Collaborator Author

cdce8p commented Jun 7, 2023

@JelleZijlstra This is the next one for PEP 696. Fairly small. The primer result is as expected.

@JelleZijlstra
Copy link
Member

I think we actually don't want defaults to be used for functions? https://peps.python.org/pep-0696/#function-defaults

cc @Gobot1234

@Gobot1234
Copy link
Contributor

I think the best way to deal with defaults in functions is to treat them as just normal generic functions without a default (for now) just for reusability and the error should be when dealing with unions etc. (pyright here has the correct behaviour)
image

@cdce8p
Copy link
Collaborator Author

cdce8p commented Jun 7, 2023

I actually used the pyright test case here to implement it. Basically the default is ignored unless mypy would return Any.
https://github.com/microsoft/pyright/blob/51d2ff8beb19b60e21732a11e75bb5c6514e62ac/packages/pyright-internal/src/tests/samples/typeVarDefaultFunction1.py

I think the best way to deal with defaults in functions is to treat them as just normal generic functions without a default (for now) just for reusability and the error should be when dealing with unions etc. (pyright here has the correct behaviour)
image

Mypy detects this as well

Incompatible return value type (got "Union[str, T1]", expected "T1")

@cdce8p cdce8p force-pushed the TypeVar-03-apply-functions branch from 85eb940 to c58532e Compare June 15, 2023 08:58
@cdce8p cdce8p changed the title Apply TypeVar defaults to functions + additional validation (PEP 696) Apply TypeVar defaults to functions (PEP 696) Jun 15, 2023
@cdce8p
Copy link
Collaborator Author

cdce8p commented Jun 15, 2023

Moved the validation portion to a separate PR #15442 as that likely uncontroversial.

Although I still think, this PR is the correct move as it matches mypy prior behavior and the way pyright handles it. The default is only used as fallback / last resort instead of Any for functions.

@github-actions

This comment has been minimized.

@cdce8p cdce8p force-pushed the TypeVar-03-apply-functions branch from c58532e to 3d42c57 Compare June 15, 2023 14:24
@github-actions

This comment has been minimized.

@cdce8p cdce8p force-pushed the TypeVar-03-apply-functions branch from 3d42c57 to 1874722 Compare June 16, 2023 13:09
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

steam.py (https://github.com/Gobot1234/steam.py)
- steam/id.py:576: error: Need type annotation for "id"  [var-annotated]
- steam/app.py:195: error: Need type annotation for "app"  [var-annotated]
- steam/app.py:207: error: Need type annotation for "licenses"  [var-annotated]
- steam/app.py:1435: error: Need type annotation for "partial_dlc"  [var-annotated]
- steam/app.py:1447: error: List comprehension has incompatible type List[PartialPackage[<nothing>]]; expected List[FetchedAppPackage]  [misc]
+ steam/app.py:1447: error: List comprehension has incompatible type List[PartialPackage[str | None]]; expected List[FetchedAppPackage]  [misc]
- steam/package.py:357: error: Need type annotation for "master_package"  [var-annotated]
- steam/game_server.py:319: error: Need type annotation for "app"  [var-annotated]
- steam/profile.py:119: error: Need type annotation for "app"  [var-annotated]
- steam/profile.py:144: error: Need type annotation for "msg"  [var-annotated]
- steam/manifest.py:552: error: Need type annotation for "app"  [var-annotated]
- steam/manifest.py:967: error: Need type annotation for "parent"  [var-annotated]
- steam/manifest.py:1126: error: Need type annotation for "_apps"  [var-annotated]
+ steam/manifest.py:1158: error: Incompatible return value type (got "list[PartialApp[str | None]]", expected "list[PartialApp[None]] | list[PartialApp[str]]")  [return-value]
- steam/user.py:266: error: Need type annotation for "new_message"  [var-annotated]
- steam/published_file.py:210: error: Need type annotation for "created_with"  [var-annotated]
- steam/event.py:77: error: Need type annotation for "app"  [var-annotated]
- steam/event.py:276: error: Incompatible types in assignment (expression has type "App[Any] | None", variable has type "PartialApp[Any] | None")  [assignment]
+ steam/event.py:276: error: Incompatible types in assignment (expression has type "App[Any] | PartialApp[str | None] | None", variable has type "PartialApp[str | None] | None")  [assignment]
- steam/clan.py:542: error: Need type annotation for "event"  [var-annotated]
- steam/state.py:304: error: Need type annotation for "steam_id"  [var-annotated]
- steam/state.py:304: error: Argument "type" to "ID" has incompatible type "int"; expected "None"  [arg-type]
+ steam/state.py:304: error: Argument "type" to "ID" has incompatible type "int"; expected "Type | None"  [arg-type]
- steam/state.py:617: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:677: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:700: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:721: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:731: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:739: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:777: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:788: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:801: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:1388: error: Need type annotation for "id"  [var-annotated]
- steam/state.py:1430: error: Incompatible types in assignment (expression has type "ID[Any]", variable has type "Clan")  [assignment]
+ steam/state.py:1430: error: Incompatible types in assignment (expression has type "Clan | ID[Type]", variable has type "Clan")  [assignment]
- steam/state.py:1432: error: Incompatible types in assignment (expression has type "ID[Any]", variable has type "Clan")  [assignment]
+ steam/state.py:1432: error: Incompatible types in assignment (expression has type "ID[Type]", variable has type "Clan")  [assignment]
- steam/state.py:1720: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:1813: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:1852: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:1872: error: Need type annotation for "steam_id"  [var-annotated]
- steam/state.py:1915: error: Need type annotation for "license_"  [var-annotated]
- steam/state.py:2352: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:2359: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:2377: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:2388: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:2398: error: Need type annotation for "msg"  [var-annotated]
- steam/state.py:2418: error: Need type annotation for "msg"  [var-annotated]
- steam/bundle.py:98: error: Need type annotation for "_apps"  [var-annotated]
- steam/bundle.py:99: error: Need type annotation for "_packages"  [var-annotated]
- steam/store.py:75: error: Need type annotation for "package"  [var-annotated]
- steam/store.py:76: error: Need type annotation for "bundle"  [var-annotated]
- steam/store.py:171: error: Need type annotation for "_apps"  [var-annotated]
- steam/store.py:186: error: Need type annotation for "related_parent_app"  [var-annotated]
- steam/store.py:327: error: Need type annotation for "package"  [var-annotated]
- steam/client.py:572: error: Need type annotation for "steam_id"  [var-annotated]
- steam/client.py:572: error: Argument "type" to "ID" has incompatible type "int"; expected "None"  [arg-type]
+ steam/client.py:572: error: Argument "type" to "ID" has incompatible type "int"; expected "Type | None"  [arg-type]
- steam/client.py:646: error: Need type annotation for "steam_id"  [var-annotated]
- steam/client.py:646: error: Argument "type" to "ID" has incompatible type "int"; expected "None"  [arg-type]
+ steam/client.py:646: error: Argument "type" to "ID" has incompatible type "int"; expected "Type | None"  [arg-type]
- steam/client.py:658: error: Need type annotation for "steam_id"  [var-annotated]
- steam/client.py:658: error: Argument "type" to "ID" has incompatible type "int"; expected "None"  [arg-type]
+ steam/client.py:658: error: Argument "type" to "ID" has incompatible type "int"; expected "Type | None"  [arg-type]
- steam/client.py:1154: error: Need type annotation for "trades"  [var-annotated]

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

@hauntsaninja hauntsaninja merged commit 9dbb123 into python:master Aug 12, 2023
@cdce8p cdce8p deleted the TypeVar-03-apply-functions branch August 12, 2023 09:05
@cdce8p cdce8p added the topic-pep-696 TypeVar defaults label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-pep-696 TypeVar defaults
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants