Skip to content

Disallow item access of NotRequired TypedDict entries. #14717

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Disallow item access of NotRequired TypedDict entries. #14717

wants to merge 2 commits into from

Conversation

bsmedberg-xometry
Copy link

Disallow direct item access of NotRequired TypedDict properties: these should

always be accessed through .get() because the keys may not be present.

Fixes #12094

Test Plan
Includes unit test coverage for both item access (getitem as an rvalue) and item setting (setitem as an lvalue).

Two existing tests required updates because they partially exercized the now-forbidden codepath using reveal_type.

This PR is a replacement of #12095 which was too large and had bitrotted.

@bsmedberg-xometry
Update documentation for more clarity around non-required TypedDict i…
3d78a2f 
…tems.
@bsmedberg-xometry
1. Disallow direct item access of NotRequired TypedDict properties
d54d25a 
2. When using .get() on a typeddict, the result type will now be a union of the dict[key] type and the type of the default parameter, instead of `object`

Fixes python#12094 - replaces python#12095 which is now bitrotted
@bsmedberg-xometry bsmedberg-xometry mentioned this pull request Feb 16, 2023
Closed
@bsmedberg-xometry
Copy link
Author

@davidfstr FYI and your review please. I believe I've addressed the issues from the previous PR here:

  • remove all the type narrowing logic and related tests, which are considered incorrect
  • accepted code review suggestions in mypy/plugins/default.py

The consequence of the removal of type narrowing is that the following code will not typecheck:

class A(TypedStruct):
    t: NotRequired["str"]

def get_a_t(a: A) -> str:
    if "t" in a:
        return a["t"]
    return "unknown"

In order for this code to typecheck, you've have to rewrite it to use .get():

def get_a_t(a: A) -> str:
    t = a.get("t")
    if t is not None:
        return t
    return "unknown"

@github-actions
Copy link
Contributor

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

python-chess (https://github.com/niklasf/python-chess)
+ chess/engine.py:1812: error: TypedDict "InfoDict" key "currline" is not required and might not be present.  [typeddict-item-access]
+ chess/engine.py:1828: error: TypedDict "InfoDict" key "refutation" is not required and might not be present.  [typeddict-item-access]
+ chess/engine.py:2329: error: TypedDict "InfoDict" key "score" is not required and might not be present.  [typeddict-item-access]

paasta (https://github.com/yelp/paasta)
+ paasta_tools/instance/hpa_metrics_parser.py:35: error: TypedDict "HPAMetricsDict" key "target_value" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/instance/hpa_metrics_parser.py:60: error: TypedDict "HPAMetricsDict" key "current_value" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2090: error: TypedDict "SystemPaastaConfigDict" key "zookeeper" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2108: error: TypedDict "SystemPaastaConfigDict" key "docker_registry" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2121: error: TypedDict "SystemPaastaConfigDict" key "hacheck_sidecar_volumes" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2135: error: TypedDict "SystemPaastaConfigDict" key "volumes" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2147: error: TypedDict "SystemPaastaConfigDict" key "cluster" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2154: error: TypedDict "SystemPaastaConfigDict" key "dashboard_links" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2157: error: TypedDict "SystemPaastaConfigDict" key "cr_owners" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2173: error: TypedDict "SystemPaastaConfigDict" key "api_endpoints" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2243: error: TypedDict "SystemPaastaConfigDict" key "log_writer" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/utils.py:2256: error: TypedDict "SystemPaastaConfigDict" key "log_reader" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/secret_tools.py:218: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/secret_tools.py:224: error: TypedDict "SecretVolume" key "container_path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/secret_tools.py:224: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/secret_tools.py:228: error: TypedDict "SecretVolume" key "items" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/secret_tools.py:235: error: TypedDict "SecretVolumeItem" key "key" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/secret_tools.py:240: error: TypedDict "SecretVolume" key "container_path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/secret_tools.py:240: error: TypedDict "SecretVolumeItem" key "path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/long_running_service_tools.py:218: error: TypedDict "LongRunningServiceConfigDict" key "nerve_ns" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/drain_lib.py:232: error: TypedDict "SpoolInfo" key "state" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/drain_lib.py:236: error: TypedDict "SpoolInfo" key "state" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/drain_lib.py:308: error: TypedDict "UrlSpec" key "url_format" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/drain_lib.py:325: error: TypedDict "UrlSpec" key "success_codes" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/drain_lib.py:328: error: TypedDict "UrlSpec" key "success_codes" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:696: error: TypedDict "AutoscalingParamsDict" key "decision_policy" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:707: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:709: error: TypedDict "AutoscalingParamsDict" key "setpoint" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:893: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1060: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1074: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1210: error: TypedDict "KubeContainerResourceRequest" key "cpu" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1211: error: TypedDict "KubeContainerResourceRequest" key "memory" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1212: error: TypedDict "KubeContainerResourceRequest" key "ephemeral-storage" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1376: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1380: error: TypedDict "SecretVolumeItem" key "key" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1382: error: TypedDict "SecretVolumeItem" key "path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1384: error: TypedDict "SecretVolume" key "items" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1469: error: TypedDict "SecretVolume" key "container_path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:1776: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:3635: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:3641: error: TypedDict "SecretVolume" key "container_path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:3641: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:3645: error: TypedDict "SecretVolume" key "items" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:3649: error: TypedDict "SecretVolumeItem" key "key" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:3653: error: TypedDict "SecretVolume" key "container_path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/kubernetes_tools.py:3653: error: TypedDict "SecretVolumeItem" key "path" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/setup_prometheus_adapter_config.py:194: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/setup_prometheus_adapter_config.py:210: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/setup_prometheus_adapter_config.py:225: error: TypedDict "AutoscalingParamsDict" key "setpoint" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/setup_prometheus_adapter_config.py:320: error: TypedDict "AutoscalingParamsDict" key "setpoint" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/setup_prometheus_adapter_config.py:404: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/setup_prometheus_adapter_config.py:568: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/setup_prometheus_adapter_config.py:580: error: TypedDict "AutoscalingParamsDict" key "prometheus_adapter_config" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/marathon_tools.py:710: error: TypedDict "FormattedMarathonAppDict" key "container" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/marathon_tools.py:711: error: TypedDict "FormattedMarathonAppDict" key "container" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/marathon_tools.py:766: error: TypedDict "FormattedMarathonAppDict" key "env" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/envoy_tools.py:78: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/envoy_tools.py:79: error: TypedDict "EnvoyBackend" key "port_value" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/envoy_tools.py:80: error: TypedDict "EnvoyBackend" key "eds_health_status" is not required and might not be present.  [typeddict-item-access]
+ paasta_tools/envoy_tools.py:294: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.  [typeddict-item-access]

... (truncated 36 lines) ...

operator (https://github.com/canonical/operator)
- ops/model.py:827: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "str")  [assignment]
- ops/model.py:891: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "str")  [assignment]
+ ops/charm.py:1094: error: TypedDict "_CharmMetaDict" key "maintainer" is not required and might not be present.  [typeddict-item-access]
+ ops/charm.py:1096: error: TypedDict "_CharmMetaDict" key "maintainers" is not required and might not be present.  [typeddict-item-access]
- ops/testing.py:1426: error: Value expression in dictionary comprehension has incompatible type "Union[str, int, float, None]"; expected type "Union[str, int, float]"  [misc]
- ops/testing.py:2355: error: Argument "permissions" to "FileInfo" has incompatible type "Optional[int]"; expected "int"  [arg-type]

steam.py (https://github.com/Gobot1234/steam.py)
+ steam/manifest.py:696: error: TypedDict "Depot" key "encryptedmanifests" is not required and might not be present.  [typeddict-item-access]
+ steam/manifest.py:856: error: TypedDict "Common" key "steam_release_date" is not required and might not be present.  [typeddict-item-access]
+ steam/manifest.py:873: error: TypedDict "Common" key "icon" is not required and might not be present.  [typeddict-item-access]
+ steam/manifest.py:877: error: TypedDict "Common" key "logo" is not required and might not be present.  [typeddict-item-access]
+ steam/manifest.py:882: error: TypedDict "Common" key "parent" is not required and might not be present.  [typeddict-item-access]
+ steam/manifest.py:908: error: TypedDict "Depot" key "maxsize" is not required and might not be present.  [typeddict-item-access]
+ steam/manifest.py:912: error: TypedDict "Depot" key "manifests" is not required and might not be present.  [typeddict-item-access]

cloud-init (https://github.com/canonical/cloud-init)
+ cloudinit/config/schema.py:1006: error: TypedDict "MetaSchema" key "activate_by_schema_keys" is not required and might not be present.  [typeddict-item-access]

bokeh (https://github.com/bokeh/bokeh)
+ src/bokeh/client/states.py: note: In member "run" of class "WAITING_FOR_REPLY":
+ src/bokeh/client/states.py:160:59: error: TypedDict "Header" key "reqid" is not required and might not be present.  [typeddict-item-access]
+ src/bokeh/protocol/message.py: note: In member "add_buffer" of class "Message":
+ src/bokeh/protocol/message.py:225:26: error: TypedDict "Header" key "num_buffers" is not required and might not be present.  [typeddict-item-access]

graphql-core (https://github.com/graphql-python/graphql-core)
+ src/graphql/utilities/extend_schema.py:202: error: TypedDict "GraphQLSchemaKwargs" key "types" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:211: error: TypedDict "GraphQLSchemaKwargs" key "query" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:211: error: TypedDict "GraphQLSchemaKwargs" key "mutation" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:211: error: TypedDict "GraphQLSchemaKwargs" key "subscription" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:229: error: TypedDict "GraphQLSchemaKwargs" key "directives" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:236: error: TypedDict "GraphQLSchemaKwargs" key "ast_node" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:237: error: TypedDict "GraphQLSchemaKwargs" key "extension_ast_nodes" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:267: error: TypedDict "GraphQLDirectiveKwargs" key "args" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:314: error: TypedDict "GraphQLInputObjectTypeKwargs" key "name" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:322: error: TypedDict "GraphQLInputObjectTypeKwargs" key "extension_ast_nodes" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:328: error: TypedDict "GraphQLEnumTypeKwargs" key "name" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:333: error: TypedDict "GraphQLEnumTypeKwargs" key "values" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:334: error: TypedDict "GraphQLEnumTypeKwargs" key "extension_ast_nodes" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:340: error: TypedDict "GraphQLScalarTypeKwargs" key "name" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:342: error: TypedDict "GraphQLScalarTypeKwargs" key "specified_by_url" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:350: error: TypedDict "GraphQLScalarTypeKwargs" key "extension_ast_nodes" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:376: error: TypedDict "GraphQLObjectTypeKwargs" key "name" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:385: error: TypedDict "GraphQLObjectTypeKwargs" key "extension_ast_nodes" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:413: error: TypedDict "GraphQLInterfaceTypeKwargs" key "name" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:422: error: TypedDict "GraphQLInterfaceTypeKwargs" key "extension_ast_nodes" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:436: error: TypedDict "GraphQLUnionTypeKwargs" key "name" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/extend_schema.py:442: error: TypedDict "GraphQLUnionTypeKwargs" key "extension_ast_nodes" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/build_ast_schema.py:61: error: TypedDict "GraphQLSchemaKwargs" key "ast_node" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/build_ast_schema.py:62: error: TypedDict "GraphQLSchemaKwargs" key "types" is not required and might not be present.  [typeddict-item-access]
+ src/graphql/utilities/build_ast_schema.py:75: error: TypedDict "GraphQLSchemaKwargs" key "directives" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:29: error: TypedDict "GraphQLScalarTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:36: error: TypedDict "GraphQLScalarTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:55: error: TypedDict "GraphQLObjectTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:86: error: TypedDict "GraphQLObjectTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:107: error: TypedDict "GraphQLInterfaceTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:138: error: TypedDict "GraphQLInterfaceTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:148: error: TypedDict "GraphQLUnionTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:157: error: TypedDict "GraphQLUnionTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:167: error: TypedDict "GraphQLEnumTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:168: error: TypedDict "GraphQLEnumValueKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:184: error: TypedDict "GraphQLEnumTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:185: error: TypedDict "GraphQLEnumValueKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:197: error: TypedDict "GraphQLInputObjectTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:219: error: TypedDict "GraphQLInputObjectTypeKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:233: error: TypedDict "GraphQLDirectiveKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:234: error: TypedDict "GraphQLArgumentKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:251: error: TypedDict "GraphQLDirectiveKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:252: error: TypedDict "GraphQLArgumentKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:259: error: TypedDict "GraphQLSchemaKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_extensions.py:268: error: TypedDict "GraphQLSchemaKwargs" key "extensions" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:88: error: TypedDict "GraphQLScalarTypeKwargs" key "serialize" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:94: error: TypedDict "GraphQLScalarTypeKwargs" key "description" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:100: error: TypedDict "GraphQLScalarTypeKwargs" key "specified_by_url" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:116: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_value" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:117: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_literal" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:130: error: TypedDict "GraphQLScalarTypeKwargs" key "serialize" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:131: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_value" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:132: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_literal" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:226: error: TypedDict "GraphQLFieldKwargs" key "args" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:240: error: TypedDict "GraphQLFieldKwargs" key "description" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:246: error: TypedDict "GraphQLFieldKwargs" key "deprecation_reason" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:738: error: TypedDict "GraphQLEnumTypeKwargs" key "description" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:946: error: TypedDict "GraphQLEnumValueKwargs" key "value" is not required and might not be present.  [typeddict-item-access]
+ tests/type/test_definition.py:952: error: TypedDict "GraphQLEnumValueKwargs" key "value" is not required and might not be present.  [typeddict-item-access]

... (truncated 15 lines) ...

kornia (https://github.com/kornia/kornia)
+ kornia/feature/adalam/core.py:292: error: TypedDict "AdalamConfig" key "area_ratio" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/core.py:293: error: TypedDict "AdalamConfig" key "search_expansion" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/core.py:294: error: TypedDict "AdalamConfig" key "ransac_iters" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/core.py:295: error: TypedDict "AdalamConfig" key "min_inliers" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/core.py:296: error: TypedDict "AdalamConfig" key "min_confidence" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/core.py:297: error: TypedDict "AdalamConfig" key "orientation_difference_threshold" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/core.py:298: error: TypedDict "AdalamConfig" key "scale_rate_threshold" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/core.py:299: error: TypedDict "AdalamConfig" key "refit" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/adalam.py:226: error: TypedDict "AdalamConfig" key "scale_rate_threshold" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/adalam.py:232: error: TypedDict "AdalamConfig" key "orientation_difference_threshold" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/adalam.py:248: error: TypedDict "AdalamConfig" key "force_seed_mnn" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/adalam.py:250: error: TypedDict "AdalamConfig" key "device" is not required and might not be present.  [typeddict-item-access]
+ kornia/feature/adalam/adalam.py:260: error: TypedDict "AdalamConfig" key "device" is not required and might not be present.  [typeddict-item-access]

cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/provenance.py: note: In member "_ro_aggregates" of class "ResearchObject":
+ cwltool/provenance.py:532:44: error: TypedDict "Aggregate" key "bundledAs" is not required and might not be present.  [typeddict-item-access]

pydantic (https://github.com/samuelcolvin/pydantic)
+ pydantic/config.py:227: error: TypedDict "ConfigDict" key "extra" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:58: error: TypedDict "ConfigDict" key "title" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:59: error: TypedDict "ConfigDict" key "extra" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:60: error: TypedDict "ConfigDict" key "allow_inf_nan" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:61: error: TypedDict "ConfigDict" key "populate_by_name" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:62: error: TypedDict "ConfigDict" key "str_strip_whitespace" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:63: error: TypedDict "ConfigDict" key "str_to_lower" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:64: error: TypedDict "ConfigDict" key "str_to_upper" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:65: error: TypedDict "ConfigDict" key "strict" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:66: error: TypedDict "ConfigDict" key "ser_json_timedelta" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_generate_schema.py:67: error: TypedDict "ConfigDict" key "ser_json_bytes" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_model_construction.py:147: error: TypedDict "ConfigDict" key "undefined_types_warning" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_model_construction.py:266: error: TypedDict "ConfigDict" key "arbitrary_types_allowed" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_model_construction.py:297: error: TypedDict "ConfigDict" key "populate_by_name" is not required and might not be present.  [typeddict-item-access]
+ pydantic/_internal/_model_construction.py:315: error: TypedDict "ConfigDict" key "extra" is not required and might not be present.  [typeddict-item-access]
+ pydantic/main.py:85: error: TypedDict "ConfigDict" key "json_encoders" is not required and might not be present.  [typeddict-item-access]
+ pydantic/main.py:86: error: TypedDict "ConfigDict" key "json_encoders" is not required and might not be present.  [typeddict-item-access]
+ pydantic/main.py:91: error: TypedDict "ConfigDict" key "frozen" is not required and might not be present.  [typeddict-item-access]
+ pydantic/main.py:380: error: TypedDict "ConfigDict" key "json_dumps" is not required and might not be present.  [typeddict-item-access]
+ pydantic/dataclasses.py:275: error: TypedDict "ConfigDict" key "extra" is not required and might not be present.  [typeddict-item-access]
+ pydantic/dataclasses.py:278: error: TypedDict "ConfigDict" key "extra" is not required and might not be present.  [typeddict-item-access]
+ pydantic/dataclasses.py:291: error: TypedDict "ConfigDict" key "post_init_call" is not required and might not be present.  [typeddict-item-access]
+ pydantic/dataclasses.py:299: error: TypedDict "ConfigDict" key "post_init_call" is not required and might not be present.  [typeddict-item-access]
+ pydantic/dataclasses.py:340: error: TypedDict "ConfigDict" key "validate_assignment" is not required and might not be present.  [typeddict-item-access]

discord.py (https://github.com/Rapptz/discord.py)
- discord/automod.py:219: error: Argument "keyword_filter" to "AutoModTrigger" has incompatible type "object"; expected "Optional[List[str]]"  [arg-type]
- discord/automod.py:220: error: Argument "regex_patterns" to "AutoModTrigger" has incompatible type "object"; expected "Optional[List[str]]"  [arg-type]
- discord/automod.py:221: error: Argument "allow_list" to "AutoModTrigger" has incompatible type "object"; expected "Optional[List[str]]"  [arg-type]
- discord/automod.py:225: error: Argument 1 to "_from_value" of "ArrayFlags" has incompatible type "object"; expected "List[int]"  [arg-type]
+ discord/automod.py:225: error: Argument 1 to "_from_value" of "ArrayFlags" has incompatible type "List[<nothing>]"; expected "List[int]"  [arg-type]

... (truncated 142 lines) ...```

@gandhis1
Copy link

Pointing out a somewhat obvious problem which I'm sure some of the Mypy primer failures touch on. It is common to use the presence of one key or even one required key value to dictate the structure of the dict

So for instance, you could have two options:

class Option1(TypedDict):
    indicator: Literal["a"]
    common_key: int
    key_a: float
}

class Option2(TypedDict):
    indicator: Literal["b"]
    common_key: int
    key_b: float
}

Let's say these dicts are used interchangeably in code and function interfaces. Currently I would expect most people to type something like this as a single TypedDict using NotRequired, in which case the change suggested in this PR would cause many failures and probably excessive null guards.

The reason I created two TypedDict here is because my initial thinking would be to create a Union[Option1, Option2] and then have an if indicator == "a" guard somewhere. I know Mypy doesn't always do the most intuitive thing with unions, so I have a few questions:

  1. Will that narrow properly to make the type Option1?
  2. Let's say it does. Will it still work if you have 10 options instead of 1.
  3. Let's say it doesn't. So what would be the idiomatic way to do this that doesn't require a cumbersome and likely very verbose .get() or if ... is None check all over the place?

@bsmedberg-xometry
Copy link
Author

@gandhis1 The narrowing-by-literal case is separate, and already a mypy feature. For example the following code typechecks both before and after this PR.

from typing import TypedDict, Literal, Union

class A(TypedDict):
    kind: Literal["a"]
    a_data: int

class B(TypedDict):
    kind: Literal["b"]
    b_data: str


def f(arg: Union[A, B]) -> Union[int, str]:
    if arg["kind"] == "a":
        return arg["a_data"]
    return arg["b_data"]

This is already covered by automation at

mypy/test-data/unit/check-narrowing.test

Line 79 in ef3187a

if x5["key"] == "A":

@gandhis1
Copy link

The consequence of the removal of type narrowing is that the following code will not typecheck:

class A(TypedStruct):
    t: NotRequired["str"]

def get_a_t(a: A) -> str:
    if "t" in a:
        return a["t"]
    return "unknown"

Ok, so I glossed over this point when I was focusing on the mypy primer output.

Could you explain why this makes sense? Why is this invalid? Why should it be an error?

@bsmedberg-xometry
Copy link
Author

It is an error because the only way I figured out to make it not-an-error was the type-narrowing code which was rejected for correctness reasons. I had written code to "narrow" that NotRequired[str] into a str after the contains check, but those are not compatible types! My code would have, for instance, prevented this:

class A(TypedStruct):
    t: NotRequired["str"]

def del_a_t(a: A) -> None:
    if "t" in a:
        del a["t]

mypy currently doesn't have a mechanism (that I know about, anyway) to narrow values inside a type. So that sort of knowledge is not possible as far as I know.

@gandhis1
Copy link

I see. But as Mypy primer shows, doing this would cause a large number of new failures on existing codebases, most of which are false positives. In the absence of the proper narrowing semantics is this really worth it? Doesn't that problem need to be solved first?

It seems Pyright has had TypedDict key narrowing for a while: microsoft/pylance-release#1926

@davidfstr
Copy link
Contributor

davidfstr commented Feb 23, 2023 •
edited
Loading

(Note: I've added some comments to several of the quoted code snippets below, so they are not verbatim quotes)

The consequence of the removal of type narrowing is that the following code will not typecheck:

from typing import *

class A(TypedDict):
    t: NotRequired["str"]

def get_a_t(a: A) -> str:
    if "t" in a:
        # Want OK: This checked access should be accepted, due to type narrowing
        return a["t"]
    return "unknown"

This seems concerning. I'd expect the code above - which I suspect is a common pattern - to be accepted, even as the following code is rejected:

def get_a_t(a: A) -> str:
    # Want ERROR: mypy ideally should complain about unchecked access of a NotRequired item
    return a["t"]

And if the above code is rejected, I'd expect that it would be possible for a user to easily workaround by inserting a manual assertion that the key exists, similar to any type-narrowing assertion:

def get_a_t(a: A) -> str:
    assert "t" in a
    # Want OK
    return a["t"]

[...] I had written code to "narrow" that NotRequired[str] into a str after the contains check, but those are not compatible types! My code would have, for instance, prevented this:

def del_a_t(a: A) -> None:
    if "t" in a:
        # Want OK, but would be willing to settle for ERROR because rare to del from TypedDict
        del a["t"]

The above use of del seems like a pretty unusual thing to do to a TypedDict, so it seems okay to me for it to incorrectly mark it as an error. Assuming that use is actually unusual, the rare user that tried to do it could mark with a # type: ignore.

Have you tried Pyright on the above code to see how it behaves, since it sounds like Pyright already has some form of TypedDict key narrowing? microsoft/pylance-release#1926

@davidfstr
Copy link
Contributor

I’m also interested in investigating the kinds of new errors the current PR did create in open source code (via inspecting the mypy-primer output above) and see how hard it would be to address them. (I’ll plan to take a look myself between now and Sunday unless someone else takes the lead earlier.)

@davidfstr
Copy link
Contributor

I did examine the mypy-primer output to see what kinds of new errors this PR did create in various projects. Some patterns:

Guarded accesses, flagged incorrectly 👎

# chess/engine.py:2329
# post_info["score"] is declared as NotRequired[T]
if "score" in post_info:  
    if post_info["score"].relative >= Mate(limit.mate):
        ...

# paasta_tools/kubernetes_tools.py:1384
# secret_volume["items"] is declared as NotRequired[T]
if "items" in secret_volume:
    items = [... for item in secret_volume["items"]]

# ops/charm.py:1096
# raw['maintainer'] and raw['maintainers'] is declared as NotRequired[T]
if 'maintainer' in raw:
    self.maintainers.append(raw['maintainer'])
if 'maintainers' in raw:
    self.maintainers.extend(raw['maintainers'])

# steam/manifest.py:908
# depot["maxsize"] is declared as NotRequired[T]
max_size = int(depot["maxsize"]) if "maxsize" in depot else None

Arguably we should allow type narrowing via if-statements like if 'key' in typed_dict: and if-expressions like X if 'key' in typed_dict else Y.

Some structures declared as total=False (or NotRequired[T] on every key) too aggressively, flagged correctly 👍

# src/graphql/utilities/extend_schema.py:202
Uses:
        class GraphQLSchemaKwargs(TypedDict, total=False):
            query: Optional[GraphQLObjectType]
            mutation: Optional[GraphQLObjectType]
            subscription: Optional[GraphQLObjectType]
            types: Optional[Tuple[GraphQLNamedType, ...]]
            ...
where each key is declared as NotRequired[Optional[T]]. 
In practice it looks like the real type should be just Optional[T] instead.

# kornia/feature/adalam/core.py:292
Uses:
        class AdalamConfig(TypedDict):
            area_ratio: NotRequired[int]
            search_expansion: NotRequired[int]
            ransac_iters: NotRequired[int]
            ...
where each key is declared as NotRequired[T]. 
In practice it looks like the real type should be just T instead.

Intentional triggering of KeyError, flagged correctly IMHO 👍

# paasta_tools/utils.py:2090
    I see:
        try:
            hosts = self.config_dict["zookeeper"]
        except KeyError:
            ...
    Arguably this should be rewritten as:
        hosts = self.config_dict.get("zookeeper")
        if hosts is None:
            ...
    Or as:
        if "zookeeper" not in self.config_dict:
            ...
        hosts = self.config_dict["zookeeper"]

# steam/manifest.py:912
    I see:
        try:
            manifests = depot["manifests"]
        except KeyError:
            ...
    Arguably that should be rewritten to avoid depending on triggering KeyError.

"Fill in key if missing" 🤔

I also saw this unusual pattern:

# chess/engine.py:1812
# info["currline"] is declared as NotRequired[T]
        if "currline" not in info:
            info["currline"] = {}
        ...
        info["currline"][cpunr] = currline

Not sure if we want to flag this or not. Ideally we wouldn't.

If we did flag the assignment above as suspicious, I'd expect the ability to workaround by adding an explicit assertion:

        if "currline" not in info:
            info["currline"] = {}
        assert "currline" in info  # NEW ASSERTION
        info["currline"][cpunr] = currline

@gandhis1
Copy link

Intentional triggering of KeyError, flagged correctly IMHO

I don't think this is right. Python style guidelines consistently encourage using EAFP patterns and exceptions over LBYL.

@davidfstr
Copy link
Contributor

Python style guidelines consistently encourage using EAFP patterns and exceptions over LBYL.

(I had to look this up: EAFP versus LBYL )

At the risk of getting distracted, let me zoom out a bit to explain my opinion:

Much of typechecking seems to be attempting to eliminate unexpected runtime errors by default:

things = [1, 2]
do_stuff(thngs)  # NameError: No such name "thngs"

str_or_none = do_other_stuff()  # type: str|None
print(len(str_or_none))  # TypeError: None has no len()

stringified_int = a_function()  # type: str
print(stringified_int + 1)  # TypeError: cannot add to a str

I've always considered a KeyError to be an unexpected runtime error, similar in form to a NameError (which I always treat as unexpected). Thus from my point of view, any line that a type checker identifies as possibly raising a KeyError in normal operation is suspect and worth flagging.

@gandhis1
Copy link

I agree it should be an error if the potential KeyError is unhandled. But if it is handled - then it shouldn't be.

@ikonst
Copy link
Contributor

ikonst commented Mar 9, 2023 •
edited
Loading

For d: dict, mypy doesn't flag d["key"] as an error, so generally speaking it doesn't deem KeyError as an "unexpected runtime error" (nor IndexError for lists).

Given

my_tuple: tuple[int] = (42,)
my_list: list[int] = []

class C:
  attr: int

class TD(TypedDict):
  key: NotRequired[int]

then, consistently, mypy will

  • flag td["otherKey"] like it'd flag C().other_attr or my_tuple[1]
  • deem td["key"] and C().attr to be int
  • not flag td["key"] like it won't flag C().attr (despite NameError) or my_list[0]

In some cases, td.get("key") might not be equivalent due to the value itself being optional, and I think we should at minimum have td.get("key", my_sentinel) and "key" in td both work as guards, for us to implement such a rule. But even then it might not be consistent with how other lookups are checked.

@davidfstr
Copy link
Contributor

Returning to the original topic, the next steps to move this PR forward IMHO are to:

@bsmedberg-xometry
Copy link
Author

Thank you all. I do not think that I can accomplish the additional fix of guarded accesses (at least without learning much more about the mypy value narrowing system than I can afford to do). I'm happy for anyone who does understand value narrowing to take this and run with it.

@davidfstr
Copy link
Contributor

Thanks for your efforts @bsmedberg-xometry ! I'll follow up on the original issue to solicit further implementation support.

@davidfstr davidfstr mentioned this pull request Mar 18, 2023
Open
@AlexWaygood AlexWaygood mentioned this pull request Jun 4, 2024
Closed
@altaha
Copy link

altaha commented Nov 18, 2024

Any update on this? I found it quite surprising that mypy did not flag direct unchecked access to a NotRequired field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
topic-typed-dict
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Disallow x["foo"] for NotRequired TypedDict access
6 participants
@bsmedberg-xometry @gandhis1 @davidfstr @ikonst @altaha @AlexWaygood