Disallow direct item access of NotRequired TypedDict properties: … #12095
Conversation
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
…e should always be accessed through .get() because the keys may not be present. Fixes python#12094
|
cc @davidfstr |
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
|
I will come back to this next week to fix .get(), but I think I also need advice on whether this will adversely affect the python ecosystem. |
1. invalid keys type-check as the default value type (None or the provided default) 2. required keys type-check just to the declared type, never the fallback 3. optional keys type-check to the declared type or the default value type Fixes tests to match the better typecheck results.
This comment has been minimized.
This comment has been minimized.
|
One of the errors in https://github.com/Gobot1234/steam.py is on this line: self.colour = int(data["name_color"], 16) if "name_color" in data else Nonebut surely that is valid? |
|
That's a good point! From an abstract type-checking perspective, the way I'd want to solve that is with a narrowing pass: However, I've not even looked at the way narrowing and inference works in mypy so I'd have to do some research on whether that is a huge or small engineering task. |
If I recall correctly, type narrowing happens for if-statements but not for x-if-y-else-z ternary expressions (but probably should). Assuming that's true, the error could be avoided for now by rewriting as: if "name_color" in data:
self.colour = int(data["name_color"], 16)
else:
self.colour = NoneType narrowing in ternary expressions I expect to be sufficiently difficult to implement that you would want a separate PR for that, if you were to pursue adding that support as well. |
…ral' in typed_dict`.
Also document the new and more-ergonomic way to mix required and not-required items in a TypedDict using the NotRequired annotation.
|
@davidfstr roger, understood. The new commits I've just added implement the narrowing logic (with a test). I've also updated the documentation to match the new proposed behavior and document the more ergonomic NotRequired method of constructing TypedDict with mixed required/not-required elements. |
davidfstr
reviewed
Feb 2, 2022
Comment on lines
+265
to
+276
| # It would be nice to issue a "TypedDict has no key {key}" failure here. However, | ||
| # we don't do this because in the case where you have a union of typeddicts, and | ||
| # one of them has the key but others don't, an error message is incorrect, and | ||
| # the plugin API has no mechanism to distinguish these cases. | ||
| output_types.append(default_type or NoneType()) | ||
| continue | ||
|
|
||
| if ctx.type.is_required(key): | ||
| # Without unions we could issue an error for .get('required_key', default) because | ||
| # the default doesn't make sense. But because of unions, we don't do that. | ||
| output_types.append(value_type) | ||
| continue |
There was a problem hiding this comment.
This looks a bit unusual to me. However I am unfamilar with mypy's plugin system - used by this file - so can't comment on the workaround.
Someone else familar with mypy plugins may want to look over this code more carefully.
There was a problem hiding this comment.
@JukkaL , it looks like you've made all previous notable modifications to typed_dict_get_callback and may have designed the plugin system.
Could you take a look at this code yourself, or suggest a different reviewer who would be familiar with the limits of mypy's plugin system?
There was a problem hiding this comment.
Did finish my own review of typed_dict_get_callback here. Further review (by JukkaL or others) is no longer requested.
test-data/unit/check-literal.test
Outdated
Comment on lines
2224
to
2232
| reveal_type(d[a_key]) # N: Revealed type is "builtins.int" | ||
| reveal_type(d[b_key]) # N: Revealed type is "builtins.str" | ||
| reveal_type(d[b_key]) # N: Revealed type is "builtins.str" \ | ||
| # E: TypedDict "Outer" key "b" is not required. | ||
| reveal_type(d.get(b_key)) # N: Revealed type is "builtins.str" | ||
| d[c_key] # E: TypedDict "Outer" has no key "c" | ||
|
|
||
| reveal_type(d.get(a_key, u)) # N: Revealed type is "Union[builtins.int, __main__.Unrelated]" | ||
| reveal_type(d.get(a_key, u)) # N: Revealed type is "builtins.int" | ||
| reveal_type(d.get(b_key, u)) # N: Revealed type is "Union[builtins.str, __main__.Unrelated]" | ||
| reveal_type(d.get(c_key, u)) # N: Revealed type is "builtins.object" | ||
| reveal_type(d.get(c_key, u)) # N: Revealed type is "__main__.Unrelated" |
There was a problem hiding this comment.
Nice to see better inference happening here. 🎉 (No changes requested.)
davidfstr
reviewed
Feb 2, 2022
There was a problem hiding this comment.
The new commits I've just added implement the narrowing logic (with a test). I've also updated the documentation to match the new proposed behavior and document the more ergonomic NotRequired method of constructing TypedDict with mixed required/not-required elements.
Very cool. Did review these new commits as well.
Comment on lines
1118
to
+1129
| Mixing required and non-required items | ||
| -------------------------------------- | ||
|
|
||
| In addition to allowing reuse across ``TypedDict`` types, inheritance also allows | ||
| you to mix required and non-required (using ``total=False``) items | ||
| in a single ``TypedDict``. Example: | ||
| When a ``TypedDict`` has a mix of items that are required and not required, | ||
| the ``NotRequired`` type annotation can be used to specify this for each field: | ||
|
|
||
| .. code-block:: python | ||
|
|
||
| class MovieBase(TypedDict): | ||
| class Movie(TypedDict): | ||
| name: str | ||
| year: int | ||
|
|
||
| class Movie(MovieBase, total=False): | ||
| based_on: str | ||
| based_on: NotRequired[str] |
There was a problem hiding this comment.
This documentation is so much more clear with this change. 👍 (No changes requested.)
| @@ -4865,6 +4860,56 @@ def replay_lookup(new_parent_type: ProperType) -> Optional[Type]: | |||
| expr = parent_expr | |||
| expr_type = output[parent_expr] = make_simplified_union(new_parent_types) | |||
|
|
|||
| def refine_optional_in(self, | |||
There was a problem hiding this comment.
The narrowing support provided by this commit is very cool, but I don't have enough brainpower this evening to look at all the new code in this commit in detail.
Requesting a second pair of eyes on the narrowing logic in this commit.
There was a problem hiding this comment.
Did review the narrowing support here. Additional review is no longer requested for this commit.
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: graphql-core (https://github.com/graphql-python/graphql-core)
- src/graphql/utilities/extend_schema.py:644: error: Unused "type: ignore" comment
+ src/graphql/utilities/extend_schema.py:204: error: TypedDict "GraphQLDirectiveKwargs" key "args" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:239: error: TypedDict "GraphQLInputObjectTypeKwargs" key "name" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:252: error: TypedDict "GraphQLInputObjectTypeKwargs" key "fields" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:256: error: TypedDict "GraphQLInputObjectTypeKwargs" key "extension_ast_nodes" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:262: error: TypedDict "GraphQLEnumTypeKwargs" key "name" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:267: error: TypedDict "GraphQLEnumTypeKwargs" key "values" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:268: error: TypedDict "GraphQLEnumTypeKwargs" key "extension_ast_nodes" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:274: error: TypedDict "GraphQLScalarTypeKwargs" key "name" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:276: error: TypedDict "GraphQLScalarTypeKwargs" key "specified_by_url" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:284: error: TypedDict "GraphQLScalarTypeKwargs" key "extension_ast_nodes" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:291: error: TypedDict "GraphQLObjectTypeKwargs" key "name" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:298: error: TypedDict "GraphQLObjectTypeKwargs" key "interfaces" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:304: error: TypedDict "GraphQLObjectTypeKwargs" key "fields" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:308: error: TypedDict "GraphQLObjectTypeKwargs" key "extension_ast_nodes" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:315: error: TypedDict "GraphQLInterfaceTypeKwargs" key "name" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:322: error: TypedDict "GraphQLInterfaceTypeKwargs" key "interfaces" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:328: error: TypedDict "GraphQLInterfaceTypeKwargs" key "fields" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:332: error: TypedDict "GraphQLInterfaceTypeKwargs" key "extension_ast_nodes" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:338: error: TypedDict "GraphQLUnionTypeKwargs" key "name" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:345: error: TypedDict "GraphQLUnionTypeKwargs" key "types" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:348: error: TypedDict "GraphQLUnionTypeKwargs" key "extension_ast_nodes" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:635: error: TypedDict "GraphQLSchemaKwargs" key "types" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:661: error: TypedDict "GraphQLSchemaKwargs" key "directives" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:668: error: TypedDict "GraphQLSchemaKwargs" key "ast_node" is not required and might not be present.
+ src/graphql/utilities/extend_schema.py:669: error: TypedDict "GraphQLSchemaKwargs" key "extension_ast_nodes" is not required and might not be present.
+ src/graphql/utilities/build_ast_schema.py:61: error: TypedDict "GraphQLSchemaKwargs" key "ast_node" is not required and might not be present.
+ src/graphql/utilities/build_ast_schema.py:62: error: TypedDict "GraphQLSchemaKwargs" key "types" is not required and might not be present.
+ src/graphql/utilities/build_ast_schema.py:75: error: TypedDict "GraphQLSchemaKwargs" key "directives" is not required and might not be present.
+ tests/type/test_extensions.py:34: error: TypedDict "GraphQLScalarTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:41: error: TypedDict "GraphQLScalarTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:66: error: TypedDict "GraphQLObjectTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:97: error: TypedDict "GraphQLObjectTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:130: error: TypedDict "GraphQLInterfaceTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:161: error: TypedDict "GraphQLInterfaceTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:177: error: TypedDict "GraphQLUnionTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:186: error: TypedDict "GraphQLUnionTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:202: error: TypedDict "GraphQLEnumTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:203: error: TypedDict "GraphQLEnumValueKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:219: error: TypedDict "GraphQLEnumTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:220: error: TypedDict "GraphQLEnumValueKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:243: error: TypedDict "GraphQLInputObjectTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:265: error: TypedDict "GraphQLInputObjectTypeKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:288: error: TypedDict "GraphQLDirectiveKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:289: error: TypedDict "GraphQLArgumentKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:306: error: TypedDict "GraphQLDirectiveKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:307: error: TypedDict "GraphQLArgumentKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:320: error: TypedDict "GraphQLSchemaKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_extensions.py:329: error: TypedDict "GraphQLSchemaKwargs" key "extensions" is not required and might not be present.
+ tests/type/test_definition.py:88: error: TypedDict "GraphQLScalarTypeKwargs" key "serialize" is not required and might not be present.
+ tests/type/test_definition.py:94: error: TypedDict "GraphQLScalarTypeKwargs" key "description" is not required and might not be present.
+ tests/type/test_definition.py:100: error: TypedDict "GraphQLScalarTypeKwargs" key "specified_by_url" is not required and might not be present.
+ tests/type/test_definition.py:116: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_value" is not required and might not be present.
+ tests/type/test_definition.py:117: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_literal" is not required and might not be present.
+ tests/type/test_definition.py:130: error: TypedDict "GraphQLScalarTypeKwargs" key "serialize" is not required and might not be present.
+ tests/type/test_definition.py:131: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_value" is not required and might not be present.
+ tests/type/test_definition.py:132: error: TypedDict "GraphQLScalarTypeKwargs" key "parse_literal" is not required and might not be present.
+ tests/type/test_definition.py:286: error: TypedDict "GraphQLFieldKwargs" key "args" is not required and might not be present.
+ tests/type/test_definition.py:300: error: TypedDict "GraphQLFieldKwargs" key "description" is not required and might not be present.
+ tests/type/test_definition.py:306: error: TypedDict "GraphQLFieldKwargs" key "deprecation_reason" is not required and might not be present.
+ tests/type/test_definition.py:1111: error: TypedDict "GraphQLEnumTypeKwargs" key "description" is not required and might not be present.
+ tests/type/test_definition.py:1361: error: TypedDict "GraphQLEnumValueKwargs" key "value" is not required and might not be present.
+ tests/type/test_definition.py:1367: error: TypedDict "GraphQLEnumValueKwargs" key "value" is not required and might not be present.
+ tests/type/test_definition.py:1373: error: TypedDict "GraphQLEnumValueKwargs" key "description" is not required and might not be present.
+ tests/type/test_definition.py:1379: error: TypedDict "GraphQLEnumValueKwargs" key "deprecation_reason" is not required and might not be present.
+ tests/type/test_definition.py:1425: error: TypedDict "GraphQLInputObjectTypeKwargs" key "description" is not required and might not be present.
+ tests/type/test_definition.py:1431: error: TypedDict "GraphQLInputObjectTypeKwargs" key "out_type" is not required and might not be present.
+ tests/type/test_definition.py:1437: error: TypedDict "GraphQLInputObjectTypeKwargs" key "out_type" is not required and might not be present.
+ tests/type/test_definition.py:1659: error: TypedDict "GraphQLArgumentKwargs" key "description" is not required and might not be present.
+ tests/type/test_definition.py:1666: error: TypedDict "GraphQLArgumentKwargs" key "out_name" is not required and might not be present.
+ tests/type/test_definition.py:1672: error: TypedDict "GraphQLArgumentKwargs" key "out_name" is not required and might not be present.
+ tests/type/test_definition.py:1678: error: TypedDict "GraphQLArgumentKwargs" key "ast_node" is not required and might not be present.
+ tests/type/test_definition.py:1724: error: TypedDict "GraphQLInputFieldKwargs" key "description" is not required and might not be present.
+ tests/type/test_definition.py:1731: error: TypedDict "GraphQLInputFieldKwargs" key "out_name" is not required and might not be present.
+ tests/type/test_definition.py:1737: error: TypedDict "GraphQLInputFieldKwargs" key "out_name" is not required and might not be present.
+ tests/type/test_definition.py:1743: error: TypedDict "GraphQLArgumentKwargs" key "ast_node" is not required and might not be present.
+ tests/type/test_definition.py:1794: error: TypedDict "GraphQLInputObjectTypeKwargs" key "fields" is not required and might not be present.
+ tests/utilities/test_build_client_schema.py:628: error: TypedDict "GraphQLSchemaKwargs" key "assume_valid" is not required and might not be present.
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.
+ paasta_tools/instance/hpa_metrics_parser.py:60: error: TypedDict "HPAMetricsDict" key "current_value" is not required and might not be present.
+ paasta_tools/utils.py:2050: error: TypedDict "SystemPaastaConfigDict" key "zookeeper" is not required and might not be present.
+ paasta_tools/utils.py:2068: error: TypedDict "SystemPaastaConfigDict" key "docker_registry" is not required and might not be present.
+ paasta_tools/utils.py:2081: error: TypedDict "SystemPaastaConfigDict" key "hacheck_sidecar_volumes" is not required and might not be present.
+ paasta_tools/utils.py:2095: error: TypedDict "SystemPaastaConfigDict" key "volumes" is not required and might not be present.
+ paasta_tools/utils.py:2107: error: TypedDict "SystemPaastaConfigDict" key "cluster" is not required and might not be present.
+ paasta_tools/utils.py:2114: error: TypedDict "SystemPaastaConfigDict" key "dashboard_links" is not required and might not be present.
+ paasta_tools/utils.py:2130: error: TypedDict "SystemPaastaConfigDict" key "api_endpoints" is not required and might not be present.
+ paasta_tools/utils.py:2203: error: TypedDict "SystemPaastaConfigDict" key "log_writer" is not required and might not be present.
+ paasta_tools/utils.py:2216: error: TypedDict "SystemPaastaConfigDict" key "log_reader" is not required and might not be present.
+ paasta_tools/drain_lib.py:232: error: TypedDict "SpoolInfo" key "state" is not required and might not be present.
+ paasta_tools/drain_lib.py:236: error: TypedDict "SpoolInfo" key "state" is not required and might not be present.
+ paasta_tools/drain_lib.py:308: error: TypedDict "UrlSpec" key "url_format" is not required and might not be present.
+ paasta_tools/drain_lib.py:325: error: TypedDict "UrlSpec" key "success_codes" is not required and might not be present.
+ paasta_tools/drain_lib.py:328: error: TypedDict "UrlSpec" key "success_codes" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:643: error: TypedDict "AutoscalingParamsDict" key "decision_policy" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:654: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:656: error: TypedDict "AutoscalingParamsDict" key "setpoint" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:838: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:969: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:981: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1101: error: TypedDict "KubeContainerResourceRequest" key "cpu" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1102: error: TypedDict "KubeContainerResourceRequest" key "memory" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1103: error: TypedDict "KubeContainerResourceRequest" key "ephemeral-storage" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1264: error: TypedDict "SecretVolume" key "secret_name" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1268: error: TypedDict "SecretVolumeItem" key "key" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1270: error: TypedDict "SecretVolumeItem" key "path" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1353: error: TypedDict "SecretVolume" key "container_path" is not required and might not be present.
+ paasta_tools/kubernetes_tools.py:1635: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/setup_prometheus_adapter_config.py:193: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/setup_prometheus_adapter_config.py:209: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/setup_prometheus_adapter_config.py:223: error: TypedDict "AutoscalingParamsDict" key "setpoint" is not required and might not be present.
+ paasta_tools/setup_prometheus_adapter_config.py:315: error: TypedDict "AutoscalingParamsDict" key "setpoint" is not required and might not be present.
+ paasta_tools/setup_prometheus_adapter_config.py:399: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/setup_prometheus_adapter_config.py:562: error: TypedDict "AutoscalingParamsDict" key "metrics_provider" is not required and might not be present.
+ paasta_tools/setup_prometheus_adapter_config.py:573: error: TypedDict "AutoscalingParamsDict" key "prometheus_adapter_config" is not required and might not be present.
+ paasta_tools/marathon_tools.py:703: error: TypedDict "FormattedMarathonAppDict" key "container" is not required and might not be present.
+ paasta_tools/marathon_tools.py:704: error: TypedDict "FormattedMarathonAppDict" key "container" is not required and might not be present.
+ paasta_tools/marathon_tools.py:759: error: TypedDict "FormattedMarathonAppDict" key "env" is not required and might not be present.
+ paasta_tools/envoy_tools.py:78: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.
+ paasta_tools/envoy_tools.py:79: error: TypedDict "EnvoyBackend" key "port_value" is not required and might not be present.
+ paasta_tools/envoy_tools.py:80: error: TypedDict "EnvoyBackend" key "eds_health_status" is not required and might not be present.
+ paasta_tools/envoy_tools.py:290: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.
+ paasta_tools/envoy_tools.py:325: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.
+ paasta_tools/envoy_tools.py:326: error: TypedDict "EnvoyBackend" key "port_value" is not required and might not be present.
+ paasta_tools/envoy_tools.py:359: error: TypedDict "EnvoyBackend" key "eds_health_status" is not required and might not be present.
+ paasta_tools/envoy_tools.py:364: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.
+ paasta_tools/envoy_tools.py:364: error: TypedDict "EnvoyBackend" key "port_value" is not required and might not be present.
+ paasta_tools/envoy_tools.py:402: error: TypedDict "EnvoyBackend" key "eds_health_status" is not required and might not be present.
+ paasta_tools/deployd/common.py:106: error: TypedDict "FormattedMarathonAppDict" key "id" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:273: error: TypedDict "HaproxyBackend" key "pxname" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:305: error: TypedDict "HaproxyBackend" key "pxname" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:316: error: TypedDict "HaproxyBackend" key "status" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:368: error: TypedDict "HaproxyBackend" key "status" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:393: error: TypedDict "HaproxyBackend" key "svname" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:403: error: TypedDict "HaproxyBackend" key "pxname" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:427: error: TypedDict "HaproxyBackend" key "svname" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:463: error: TypedDict "HaproxyBackend" key "svname" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:783: error: TypedDict "HaproxyBackend" key "svname" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:795: error: TypedDict "HaproxyBackend" key "status" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:796: error: TypedDict "HaproxyBackend" key "check_status" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:797: error: TypedDict "HaproxyBackend" key "check_code" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:798: error: TypedDict "HaproxyBackend" key "lastchg" is not required and might not be present.
+ paasta_tools/smartstack_tools.py:802: error: TypedDict "HaproxyBackend" key "check_duration" is not required and might not be present.
+ paasta_tools/instance/kubernetes.py:154: error: TypedDict "HPAMetricsDict" key "name" is not required and might not be present.
+ paasta_tools/instance/kubernetes.py:160: error: TypedDict "HPAMetricsDict" key "name" is not required and might not be present.
+ paasta_tools/instance/kubernetes.py:368: error: TypedDict "EnvoyBackend" key "eds_health_status" is not required and might not be present.
+ paasta_tools/instance/kubernetes.py:371: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.
+ paasta_tools/instance/kubernetes.py:371: error: TypedDict "EnvoyBackend" key "port_value" is not required and might not be present.
+ paasta_tools/instance/kubernetes.py:405: error: TypedDict "HaproxyBackend" key "status" is not required and might not be present.
+ paasta_tools/setup_marathon_job.py:657: error: TypedDict "FormattedMarathonAppDict" key "id" is not required and might not be present.
+ paasta_tools/setup_marathon_job.py:893: error: TypedDict "FormattedMarathonAppDict" key "id" is not required and might not be present.
+ paasta_tools/check_kubernetes_services_replication.py:105: error: TypedDict "KubernetesDeploymentConfigDict" key "monitoring" is not required and might not be present.
+ paasta_tools/check_kubernetes_services_replication.py:107: error: TypedDict "KubernetesDeploymentConfigDict" key "monitoring" is not required and might not be present.
+ paasta_tools/autoscaling/autoscaling_service_lib.py:680: error: TypedDict "AutoscalingParamsDict" key "setpoint" is not required and might not be present.
+ paasta_tools/autoscaling/autoscaling_service_lib.py:830: error: TypedDict "AutoscalingParamsDict" key "decision_policy" is not required and might not be present.
+ paasta_tools/api/views/instance.py:244: error: TypedDict "FormattedMarathonAppDict" key "id" is not required and might not be present.
+ paasta_tools/api/views/instance.py:362: error: TypedDict "HaproxyBackend" key "status" is not required and might not be present.
+ paasta_tools/api/views/instance.py:394: error: TypedDict "EnvoyBackend" key "eds_health_status" is not required and might not be present.
+ paasta_tools/api/views/instance.py:397: error: TypedDict "EnvoyBackend" key "address" is not required and might not be present.
+ paasta_tools/api/views/instance.py:397: error: TypedDict "EnvoyBackend" key "port_value" is not required and might not be present.
pydantic (https://github.com/samuelcolvin/pydantic)
+ pydantic/networks.py:229: error: TypedDict "Parts" key "scheme" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:230: error: TypedDict "Parts" key "user" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:231: error: TypedDict "Parts" key "password" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:235: error: TypedDict "Parts" key "port" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:236: error: TypedDict "Parts" key "path" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:237: error: TypedDict "Parts" key "query" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:238: error: TypedDict "Parts" key "fragment" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:247: error: TypedDict "Parts" key "scheme" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:254: error: TypedDict "Parts" key "port" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:258: error: TypedDict "Parts" key "user" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:334: error: TypedDict "Parts" key "scheme" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:368: error: TypedDict "Parts" key "ipv4" is not required and might not be present. [typeddict-item-access]
+ pydantic/networks.py:368: error: TypedDict "Parts" key "ipv6" is not required and might not be present. [typeddict-item-access]
dedupe (https://github.com/dedupeio/dedupe)
+ dedupe/convenience.py:100: error: TypedDict "TrainingData" key "uncertain" is not required and might not be present.
+ dedupe/convenience.py:108: error: TypedDict "TrainingData" key "uncertain" is not required and might not be present.
kopf (https://github.com/nolar/kopf)
+ kopf/_cogs/configs/conventions.py:270: error: TypedDict "BodyEssence" key "metadata" is not required and might not be present.
+ kopf/_cogs/configs/conventions.py:279: error: TypedDict "BodyEssence" key "metadata" is not required and might not be present.
+ kopf/_cogs/configs/conventions.py:279: error: TypedDict "MetaEssence" key "annotations" is not required and might not be present.
+ kopf/_cogs/configs/conventions.py:280: error: TypedDict "BodyEssence" key "metadata" is not required and might not be present.
+ kopf/_cogs/configs/conventions.py:281: error: TypedDict "BodyEssence" key "metadata" is not required and might not be present.
+ kopf/_cogs/configs/conventions.py:281: error: TypedDict "MetaEssence" key "labels" is not required and might not be present.
+ kopf/_cogs/configs/conventions.py:282: error: TypedDict "BodyEssence" key "metadata" is not required and might not be present.
+ kopf/_cogs/configs/conventions.py:284: error: Key "metadata" of TypedDict "BodyEssence" cannot be deleted
+ kopf/_cogs/configs/conventions.py:286: error: Key "status" of TypedDict "BodyEssence" cannot be deleted
+ kopf/_cogs/clients/events.py:35: error: TypedDict "ObjectReference" key "apiVersion" is not required and might not be present.
+ kopf/_cogs/clients/events.py:35: error: TypedDict "ObjectReference" key "kind" is not required and might not be present.
+ kopf/_core/engines/peering.py:112: error: TypedDict "RawBody" key "metadata" is not required and might not be present.
+ kopf/_cogs/clients/watching.py:193: error: TypedDict "RawError" key "code" is not required and might not be present.
+ kopf/_kits/hierarchies.py:38: error: TypedDict "OwnerReference" key "uid" is not required and might not be present.
+ kopf/_kits/hierarchies.py:46: error: TypedDict "OwnerReference" key "uid" is not required and might not be present.
+ kopf/_kits/hierarchies.py:48: error: TypedDict "OwnerReference" key "apiVersion" is not required and might not be present.
+ kopf/_kits/hierarchies.py:49: error: TypedDict "OwnerReference" key "kind" is not required and might not be present.
+ kopf/_kits/hierarchies.py:50: error: TypedDict "OwnerReference" key "name" is not required and might not be present.
+ kopf/_kits/hierarchies.py:51: error: TypedDict "OwnerReference" key "uid" is not required and might not be present.
+ kopf/_kits/hierarchies.py:52: error: TypedDict "OwnerReference" key "controller" is not required and might not be present.
+ kopf/_kits/hierarchies.py:53: error: TypedDict "OwnerReference" key "blockOwnerDeletion" is not required and might not be present.
+ kopf/_kits/hierarchies.py:75: error: TypedDict "OwnerReference" key "uid" is not required and might not be present.
+ kopf/_kits/hierarchies.py:76: error: TypedDict "OwnerReference" key "uid" is not required and might not be present.
+ kopf/_kits/hierarchies.py:83: error: TypedDict "OwnerReference" key "uid" is not required and might not be present.
+ kopf/_kits/hierarchies.py:84: error: TypedDict "OwnerReference" key "uid" is not required and might not be present.
+ kopf/_core/reactor/queueing.py:108: error: TypedDict "RawBody" key "metadata" is not required and might not be present.
+ kopf/_core/reactor/queueing.py:109: error: TypedDict "RawBody" key "metadata" is not required and might not be present.
+ kopf/_core/reactor/queueing.py:114: error: TypedDict "RawBody" key "metadata" is not required and might not be present.
+ kopf/_core/reactor/queueing.py:115: error: TypedDict "RawBody" key "metadata" is not required and might not be present.
+ kopf/_core/reactor/queueing.py:116: error: TypedDict "RawBody" key "metadata" is not required and might not be present.
+ kopf/_core/reactor/observation.py:182: error: TypedDict "RawBody" key "spec" is not required and might not be present.
+ kopf/_core/reactor/observation.py:199: error: TypedDict "RawBody" key "metadata" is not required and might not be present.
+ kopf/_core/reactor/observation.py:199: error: TypedDict "RawMeta" key "name" is not required and might not be present.
+ kopf/_kits/webhooks.py:533: error: TypedDict "WebhookClientConfig" key "url" is not required and might not be present.
cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/provenance.py: note: In member "_ro_aggregates" of class "ResearchObject":
+ cwltool/provenance.py:541:44: error: TypedDict "Aggregate" key "bundledAs" is not required and might not be present. [typeddict-item-access]
zulip (https://github.com/zulip/zulip)
+ zerver/lib/streams.py:179: error: TypedDict "StreamDict" key "name" is not required and might not be present. [typeddict-item-access]
+ zerver/lib/streams.py:642: error: TypedDict "StreamDict" key "name" is not required and might not be present. [typeddict-item-access]
+ zerver/lib/streams.py:667: error: TypedDict "StreamDict" key "name" is not required and might not be present. [typeddict-item-access]
+ zerver/lib/streams.py:674: error: TypedDict "StreamDict" key "is_web_public" is not required and might not be present. [typeddict-item-access]
+ zerver/lib/streams.py:695: error: TypedDict "StreamDict" key "name" is not required and might not be present. [typeddict-item-access]
+ zerver/views/auth.py:642: error: TypedDict "ExternalAuthDataDict" key "subdomain" is not required and might not be present. [typeddict-item-access]
+ zerver/views/auth.py:650: error: TypedDict "ExternalAuthDataDict" key "subdomain" is not required and might not be present. [typeddict-item-access]
+ zerver/tests/test_auth_backends.py:1087: error: TypedDict "ExternalAuthDataD``` |
(I really like the new error message. 😁) I'm still planning to look at the new narrowing logic in mypy/checker.py a little later this week/weekend, once I find a larger block of time. |
|
When using this PR against our production codebase, I've discovered an issue with this PR, specifically about the narrowing code. This is an example testcase that now fails but shouldn't: from typing import TypedDict, NotRequired, Optional
Foo = TypedDict('Foo', {'complete': NotRequired[bool]})
def get_if_complete(foo: Foo) -> Foo:
if "complete" not in foo:
foo["complete"] = False
return foo
return fooThis testcase fails with the message
I personally disagree with the original decision-making here. Ideally I'd like to allow structural subtyping like this: class TD1(TypedDict):
a: str
b: int
class TD2(TypedDict):
a: Optional[str]
b: NotRequired[int]
td1: TD1
td2: TD2 = td1But this does somewhat alter the ergonomics and safety guarantees of the type system: because you could then do things like this: del td2["b"]
td2["a"] = NoneAnd because td2 is the same object as td1, you'd be making changes against the type contract of TD1. In most structural-subtyping systems (including typescript), you're allowed to convert from a more-specific type to the less-specific type, with the understanding that this could allow type-incorrect mutations to occur. This is a trivially-easy change to make in subtypes.py, and I have a commit ready for it, but I'd like to consider separating that into another PR (which would land first, before this one) since that could overcomplicate this one. |
|
Thanks for the PR! Since I'm going to focus on preparing the 0.940 release, I probably won't be able to review this very soon (sorry!). To other reviewers: Please don't merge this PR before I've accepted this. This change might be disruptive, but I'll need to investigate it further to get a good estimate of the impact. |
davidfstr
reviewed
Feb 9, 2022
| collection_type: Type, | ||
| ) -> Optional[Type]: | ||
| """ | ||
| Check whether a condition `optional_item in collection_type` can narrow away Optional. |
There was a problem hiding this comment.
Probably you meant:
Check whether a condition "item_type in collection_type" [...]
(There is no optional_item parameter for this docstring to refer to)
| if narrowed_left_type: | ||
| if_map = {operands[left_index]: narrowed_left_type} | ||
|
|
||
| elif right_is_narrowable: |
There was a problem hiding this comment.
If left_is_narrowable, shouldn't we still try to narrow the right side too (if right_is_narrowable)?
In particular should this just be if rather than elif?
| collection_type: Type, | ||
| ) -> Optional[Type]: | ||
| """ | ||
| Check whether a condition `'literal' in typeddict` can narrow a non-required ite |
davidfstr
reviewed
Feb 9, 2022
There was a problem hiding this comment.
Did finish reviewing mypy/plugins/default.py . Did propose a simplification of the current logic.
I have now given feedback on all files originally in this PR.
|
|
||
| default_type: Optional[Type] | ||
| if len(ctx.arg_types) == 1: | ||
| default_type = None |
There was a problem hiding this comment.
Seems like you could simplify by making this default_type = NoneType() here:
- You can then simplify the type of
default_type: Optional[Type]todefault_type: Type. - You can then simplify the
default_type or NoneType()expression later to justdefault_type. - You can (probably) then eliminate the special
if default_type is None:block.- However after eliminating that block you might need an extra length check in the following block:
if len(ctx.args[1]) == 1:, such asif len(ctx.args) >= 2 and len(ctx.args[1]) == 1.
- However after eliminating that block you might need an extra length check in the following block:
Comment on lines
+265
to
+276
| # It would be nice to issue a "TypedDict has no key {key}" failure here. However, | ||
| # we don't do this because in the case where you have a union of typeddicts, and | ||
| # one of them has the key but others don't, an error message is incorrect, and | ||
| # the plugin API has no mechanism to distinguish these cases. | ||
| output_types.append(default_type or NoneType()) | ||
| continue | ||
|
|
||
| if ctx.type.is_required(key): | ||
| # Without unions we could issue an error for .get('required_key', default) because | ||
| # the default doesn't make sense. But because of unions, we don't do that. | ||
| output_types.append(value_type) | ||
| continue |
There was a problem hiding this comment.
Did finish my own review of typed_dict_get_callback here. Further review (by JukkaL or others) is no longer requested.
I'll comment RE this thread in your new PR when I next get some more bandwidth. |
|
I'm going to get back to this at the end of the week. Expectations:
|
|
@bsmedberg-xometry , did life get busy? ^_^ |
|
Would it be helpful or welcome @bsmedberg-xometry if I helped apply feedback on this branch? The branch's diff does implement (Not)Required[] docs in mypy, which is the last piece of the (Not)Required[] functionality that is missing for mypy & CPython 3.12. I'm actively finishing (Not)Required[] functionality and so have active energy to spare toward completing this PR. |
|
@davidfstr et al: yes life and work got too busy and I dropped this. I would be happy for somebody to take this over. I recommend still as I had before: ripping out the narrowing code because #12142 was rejected (fundamentally), and that cannot work without the other. That may require some other layers of fixup. |
bsmedberg-xometry
added a commit
to xometry/mypy
that referenced
this pull request
Feb 16, 2023
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
|
I'm going to close this PR and I've opened up #14717 which unbitrots this and does the changes I've mentioned above. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
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.