-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix default of dict.get
#13222
base: main
Are you sure you want to change the base?
fix default of dict.get
#13222
Conversation
This comment has been minimized.
This comment has been minimized.
Seems like I was, in fact, missing something. That's quite a lot of noise from mypy primer. |
After a little initial investigation of the diff from Alerta:
Without this change this returns |
Just noticed that one of the diffs is from mypy itself, so that's something. |
I figured out why The current definition is this: @overload
def get(self, key: _KT, /) -> _VT | None: ...
@overload
def get(self, key: _KT, default: _VT, /) -> _VT: ...
@overload
def get(self, key: _KT, default: _T, /) -> _VT | _T: ... with I still need to work through all the diffs from mypy primer, but for right now I'm making the assumption that this is the underlying cause for all of them. If that's the case, and we want to keep the current behavior, I think we can make this change and keep the current behavior by re-ordering the overload: @overload
def get(self, key: _KT, default: _VT, /) -> _VT: ...
@overload
def get(self, key: _KT, default: None = None, /) -> _VT | None: ...
@overload
def get(self, key: _KT, default: _T, /) -> _VT | _T: ... This would keep d: dict[str, Any]
d.get("str") -> Any | None
d.get("str", None) -> Any I don't think anyone expects that explicitly adding |
I'm not quite done with going through the mypy-primer results, and I'll have a full write-up later, but this is some really weird behavior: Without this change: from typing import Any
type ConfigType = dict[str, Any]
def foo(config: ConfigType) -> None:
reveal_type(config) # note: Revealed type is "builtins.dict[builtins.str, Any]"
a = config["str"]
reveal_type(a) # note: Revealed type is "Any"
b = config.get("str", a)
reveal_type(b) # note: Revealed type is "Any" With this change: from typing import Any
type ConfigType = dict[str, Any]
def foo(config: ConfigType) -> None:
reveal_type(config) # note: Revealed type is "builtins.dict[builtins.str, Any]"
a = config["str"]
reveal_type(a) # note: Revealed type is "Any"
b = config.get("str", a)
reveal_type(b) # note: Revealed type is "Union[Any, None]" But with or without this change: from typing import Any
type ConfigType = dict[str, Any]
def foo(config: ConfigType) -> None:
reveal_type(config) # note: Revealed type is "builtins.dict[builtins.str, Any]"
a: Any = config["str"]
reveal_type(a) # note: Revealed type is "Any"
b = config.get("str", a)
reveal_type(b) # note: Revealed type is "Any" Also with or without this change: from typing import Any
def foo(config: dict[str, Any]) -> None:
reveal_type(config) # note: Revealed type is "builtins.dict[builtins.str, Any]"
a = config["str"]
reveal_type(a) # note: Revealed type is "Any"
b = config.get("str", a)
reveal_type(b) # note: Revealed type is "Any" I haven't explored the underlying cause of this yet, but it certainly feels bad. |
I created a python file which reproduces the mypy-primer diff for this change. The primer results are inline as comments. Since it's 540 lines long, I uploaded it as a gist instead of putting it here: https://gist.github.com/tungol/e792212afa4b89153b6479baa0fad3b2
|
The underlying cause of the weird behavior with a TypeAlias is that for That is: from typing import Any, reveal_type
type MyType = dict[str, Any]
d: MyType = {}
d2: dict[str, Any] = {}
a = d["str"] # this is a "special_form" Any
b = d2["str"] # this is an "explicit" Any This issue came up in python/mypy#15497 previously |
This comment has been minimized.
This comment has been minimized.
Pushing this up just to compare the mypy-primer damage as compared with python#13222. I expect it to be more disruptive, but I'm curious how much more. It seems similar to but dramatically less disruptive than an older attempt at changing dict.get in python#10294.
I'm interested in hearing if anyone has opinions about this. It's a reasonable amount of churn in primer, although I'd encourge you to ignore the alerta diff entirely. After looking at them a little more, I realized that that project is still using implicit optional arguments, and all of that would go away if that were taken into account in primer. My analysis of some parts of the diff ended up in #13224 because that milder change also triggered some of them. Ultimately after dwelling on this quite a bit, I think that I made a post on discuss.python.org which broke down significant scenarios that this affects here: https://discuss.python.org/t/dict-get-return-values-when-any-involved/73915 It hasn't seen much engagement, and I don't know if it's likely to get more at this point. Of concern separately from the mypy-primer result is a weird thing this does to pyright's inference in this case: d_str: dict[str, str] = {}
any_value: Any = object()
reveal_type(d.get("key", any_value)) # With this MR pyright says "str | None" I asked Eric about it here; He agreed that it's wonky, but isn't planning any changes to how pyright handles overloads until the formal spec is done, which is totally reasonable. If I understand his draft correctly, this would be Anyway, I know this is getting longwinded. Ultimately I don't know if this makes sense to pursue before the overload spec to gets finalized. I guess what I'd like to know from the typeshed maintainers is: a) Do you think this is in theory a good idea, or am I going in the wrong direction here? |
Diff from mypy_primer, showing the effect of this PR on open source code: pylint (https://github.com/pycqa/pylint)
+ pylint/config/utils.py:66: error: Argument "metavar" to "_CallableArgument" has incompatible type "Any | None"; expected "str" [arg-type]
alerta (https://github.com/alerta/alerta)
+ alerta/models/permission.py:29: error: Argument "match" to "Permission" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/permission.py:49: error: Argument "match" to "Permission" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/note.py:34: error: Argument "text" to "Note" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/note.py:35: error: Argument "user" to "Note" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/note.py:37: error: Argument "note_type" to "Note" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/note.py:76: error: Argument "text" to "Note" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/note.py:77: error: Argument "user" to "Note" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/note.py:79: error: Argument "note_type" to "Note" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:51: error: Argument "user" to "ApiKey" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:53: error: Argument "text" to "ApiKey" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:55: error: Argument "customer" to "ApiKey" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:89: error: Argument "user" to "ApiKey" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:91: error: Argument 1 to "type_to_scopes" of "ApiKeyHelper" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:91: error: Argument 2 to "type_to_scopes" of "ApiKeyHelper" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:92: error: Argument "text" to "ApiKey" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/key.py:93: error: Argument "expire_time" to "ApiKey" has incompatible type "Any | None"; expected "datetime" [arg-type]
+ alerta/models/key.py:96: error: Argument "customer" to "ApiKey" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/heartbeat.py:85: error: Argument "origin" to "Heartbeat" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/heartbeat.py:89: error: Argument "timeout" to "Heartbeat" has incompatible type "Any | None"; expected "int" [arg-type]
+ alerta/models/heartbeat.py:90: error: Argument "customer" to "Heartbeat" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/heartbeat.py:120: error: Argument "origin" to "Heartbeat" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/heartbeat.py:124: error: Argument "create_time" to "Heartbeat" has incompatible type "Any | None"; expected "datetime" [arg-type]
+ alerta/models/heartbeat.py:125: error: Argument "timeout" to "Heartbeat" has incompatible type "Any | None"; expected "int" [arg-type]
+ alerta/models/heartbeat.py:129: error: Argument "customer" to "Heartbeat" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:32: error: Argument "id" to "GroupUser" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:33: error: Argument "name" to "GroupUser" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:34: error: Argument "login" to "GroupUser" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:35: error: Argument "status" to "GroupUser" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:84: error: Argument "name" to "Group" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:85: error: Argument "text" to "Group" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:106: error: Argument "name" to "Group" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/group.py:107: error: Argument "text" to "Group" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/customer.py:23: error: Argument "match" to "Customer" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/customer.py:24: error: Argument "customer" to "Customer" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/customer.py:44: error: Argument "match" to "Customer" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/customer.py:45: error: Argument "customer" to "Customer" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/alert.py:98: error: Argument "resource" to "Alert" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/alert.py:99: error: Argument "event" to "Alert" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/alert.py:173: error: Argument "resource" to "Alert" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/alert.py:174: error: Argument "event" to "Alert" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/user.py:66: error: Argument "login" to "User" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/user.py:68: error: Argument "email" to "User" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/user.py:72: error: Argument "text" to "User" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/user.py:107: error: Argument "name" to "User" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/user.py:109: error: Argument "password" to "User" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/user.py:110: error: Argument "email" to "User" has incompatible type "Any | None"; expected "str" [arg-type]
+ alerta/models/user.py:116: error: Argument "text" to "User" has incompatible type "Any | None"; expected "str" [arg-type]
ibis (https://github.com/ibis-project/ibis)
- ibis/common/collections.py:280: error: Definition of "get" in base class "dict" is incompatible with definition in base class "Mapping" [misc]
werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/wsgi.py:78: error: Argument 1 to "int" has incompatible type "Optional[Any]"; expected "Union[str, Buffer, SupportsInt, SupportsIndex, SupportsTrunc]" [arg-type]
spark (https://github.com/apache/spark)
+ python/pyspark/sql/session.py:322: error: Dict entry 0 has incompatible type "str": "Any | None"; expected "str": "str" [dict-item]
+ python/pyspark/sql/session.py:323: error: Dict entry 1 has incompatible type "str": "Any | None"; expected "str": "str" [dict-item]
+ python/pyspark/sql/session.py:574: error: Item "None" of "Any | None" has no attribute "startswith" [union-attr]
streamlit (https://github.com/streamlit/streamlit)
+ lib/streamlit/runtime/state/session_state.py: note: In member "get_id_from_key" of class "KeyIdMapper":
+ lib/streamlit/runtime/state/session_state.py:314:9: error: Returning Any from function declared to return "str" [no-any-return]
pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/apply.py:1560: error: Dict entry 0 has incompatible type "str": "Any | None"; expected "str": "str | bool" [dict-item]
+ pandas/core/apply.py:1560: error: Dict entry 1 has incompatible type "str": "Any | None"; expected "str": "str | bool" [dict-item]
prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/locking/memory.py:196: note: def get(self, Never, /) -> None
+ src/prefect/locking/memory.py:196: note: def get(self, Never, None = ..., /) -> None
- src/prefect/locking/memory.py:207: note: def get(self, Never, /) -> None
+ src/prefect/locking/memory.py:207: note: def get(self, Never, None = ..., /) -> None
+ src/prefect/server/orchestration/core_policy.py:221: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
+ src/prefect/server/orchestration/core_policy.py:223: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
+ src/prefect/server/orchestration/core_policy.py:235: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
+ src/prefect/server/orchestration/core_policy.py:237: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
+ src/prefect/server/api/concurrency_limits.py:201: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
+ src/prefect/server/api/concurrency_limits.py:203: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
+ src/prefect/server/api/concurrency_limits.py:215: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
+ src/prefect/server/api/concurrency_limits.py:217: error: Item "None" of "Any | None" has no attribute "active_slots" [union-attr]
- src/prefect/runner/runner.py:859: note: def get(self, Never, /) -> None
+ src/prefect/runner/runner.py:859: note: def get(self, Never, None = ..., /) -> None
- src/prefect/records/memory.py:176: note: def get(self, Never, /) -> None
+ src/prefect/records/memory.py:176: note: def get(self, Never, None = ..., /) -> None
steam.py (https://github.com/Gobot1234/steam.py)
- steam/ext/commands/utils.py:43: note: def get(self, str, /) -> _VT | None
+ steam/ext/commands/utils.py:43: note: def get(self, str, None = ..., /) -> _VT | None
operator (https://github.com/canonical/operator)
- ops/_private/harness.py:1193: error: Argument 2 to "get" of "dict" has incompatible type "None"; expected "Mapping[str, str]" [arg-type]
+ ops/_private/harness.py:1193: error: Incompatible return value type (got "dict[str, str] | None", expected "Mapping[str, str]") [return-value]
altair (https://github.com/vega/altair)
+ altair/utils/_vegafusion_data.py:230: error: Argument 1 to "handle_row_limit_exceeded" has incompatible type "Any | None"; expected "int" [arg-type]
+ altair/utils/_vegafusion_data.py:281: error: Argument 1 to "handle_row_limit_exceeded" has incompatible type "Any | None"; expected "int" [arg-type]
freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/entryexitanalysis.py:365: error: Argument 1 to "Path" has incompatible type "Any | None"; expected "str | PathLike[str]" [arg-type]
mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/io/compat.py:510: error: Argument 1 to "tuple" has incompatible type "Any | None"; expected "Iterable[Any]" [arg-type]
+ mitmproxy/proxy/events.py:97: error: Invalid index type "Any | None" for "dict[Command, type[CommandCompleted]]"; expected type "Command" [index]
beartype (https://github.com/beartype/beartype)
+ beartype/_check/error/errcause.py:552: error: Incompatible types in assignment (expression has type "Callable[[ViolationCause], ViolationCause] | None", variable has type "Callable[[ViolationCause], ViolationCause]") [assignment]
+ beartype/_check/error/errcause.py:553: error: Unused "type: ignore" comment [unused-ignore]
ignite (https://github.com/pytorch/ignite)
+ ignite/handlers/tqdm_logger.py:216: error: Argument 1 to "_OutputHandler" has incompatible type "Any | None"; expected "str" [arg-type]
kornia (https://github.com/kornia/kornia)
+ kornia/augmentation/container/augment.py:504: error: Incompatible types in assignment (expression has type "Any | None", variable has type "list[DataKey]") [assignment]
httpx-caching (https://github.com/johtso/httpx-caching)
+ httpx_caching/_sync/_cache.py:20: error: Argument 1 to "loads" of "Serializer" has incompatible type "Any | None"; expected "bytes" [arg-type]
+ httpx_caching/_async/_cache.py:20: error: Argument 1 to "loads" of "Serializer" has incompatible type "Any | None"; expected "bytes" [arg-type]
openlibrary (https://github.com/internetarchive/openlibrary)
+ openlibrary/solr/updater/edition.py: note: In member "number_of_pages" of class "EditionSolrBuilder":
+ openlibrary/solr/updater/edition.py:167: error: Argument 1 to "int" has incompatible type "Any | None"; expected "str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc" [arg-type]
mypy (https://github.com/python/mypy)
+ mypy/partially_defined.py:510: error: Never apply isinstance() to unexpanded types; use mypy.types.get_proper_type() first [misc]
+ mypy/partially_defined.py:510: note: If you pass on the original type after the check, always use its unexpanded version
sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/domains/python/_object.py: note: In member "add_target_and_index" of class "PyObject":
+ sphinx/domains/python/_object.py:349:45: error: Argument 1 to "get_index_text" of "PyObject" has incompatible type "Any | None"; expected "str" [arg-type]
+ sphinx/domains/javascript.py: note: In member "handle_signature" of class "JSObject":
+ sphinx/domains/javascript.py:132:16: error: Incompatible return value type (got "tuple[str, Any | None]", expected "tuple[str, str]") [return-value]
cloud-init (https://github.com/canonical/cloud-init)
+ cloudinit/config/cc_ca_certs.py:256: error: Unsupported right operand type for in ("Any | None") [operator]
+ cloudinit/config/cc_ca_certs.py:262: error: Item "None" of "Any | None" has no attribute "get" [union-attr]
+ cloudinit/config/cc_ca_certs.py:263: error: Item "None" of "Any | None" has no attribute "get" [union-attr]
+ cloudinit/config/cc_ca_certs.py:269: error: Unsupported right operand type for in ("Any | None") [operator]
django-stubs (https://github.com/typeddjango/django-stubs)
+ django-stubs/utils/datastructures.pyi:65: error: Signature of "get" incompatible with supertype "dict" [override]
+ django-stubs/utils/datastructures.pyi:65: note: Superclass:
+ django-stubs/utils/datastructures.pyi:65: note: @overload
+ django-stubs/utils/datastructures.pyi:65: note: def get(self, _K, None = ..., /) -> Optional[_V]
+ django-stubs/utils/datastructures.pyi:65: note: @overload
+ django-stubs/utils/datastructures.pyi:65: note: def get(self, _K, _V, /) -> _V
+ django-stubs/utils/datastructures.pyi:65: note: @overload
+ django-stubs/utils/datastructures.pyi:65: note: def [_T] get(self, _K, _T, /) -> Union[_V, _T]
+ django-stubs/utils/datastructures.pyi:65: note: Subclass:
+ django-stubs/utils/datastructures.pyi:65: note: @overload
+ django-stubs/utils/datastructures.pyi:65: note: def get(self, key: _K) -> Optional[_V]
+ django-stubs/utils/datastructures.pyi:65: note: @overload
+ django-stubs/utils/datastructures.pyi:65: note: def [_Z] get(self, key: _K, default: _Z) -> Union[_V, _Z]
dd-trace-py (https://github.com/DataDog/dd-trace-py)
+ ddtrace/appsec/_iast/taint_sinks/path_traversal.py:30: error: Argument "evidence_value" to "report" of "VulnerabilityBase" has incompatible type "Any | None"; expected "str" [arg-type]
core (https://github.com/home-assistant/core)
+ homeassistant/components/alexa/capabilities.py:1442: error: Unsupported right operand type for in ("Any | None") [operator]
+ homeassistant/components/tradfri/config_flow.py:57: error: Argument 2 to "authenticate" has incompatible type "Any | None"; expected "str" [arg-type]
+ homeassistant/components/fints/sensor.py:86: error: Argument 2 to "FinTsClient" has incompatible type "Any | None"; expected "str" [arg-type]
+ homeassistant/components/everlights/light.py:123: error: Value of type "Any | None" is not indexable [index]
+ homeassistant/components/everlights/light.py:123: error: Unsupported operand types for / ("None" and "int") [operator]
+ homeassistant/components/everlights/light.py:123: note: Left operand is of type "Any | None"
+ homeassistant/components/melcloud/climate.py:229: error: Argument 1 to "_apply_set_hvac_mode" of "AtaDeviceClimate" has incompatible type "Any | None"; expected "HVACMode" [arg-type]
+ homeassistant/components/home_connect/light.py:204: error: Argument 2 to "brightness_to_value" has incompatible type "Any | None"; expected "int" [arg-type]
|
This seems like a straightforward fix for a relatively important class/method, so I'm wondering if I'm missing something.