Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test suite characterizing dict.get() #13225

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Dec 10, 2024

While working on understanding the subtleties of dict.get(), I read this comment from Jelle:

It would also be good to add some more test cases to cover dict.get; it's been a very tricky function to cover in the stubs.

Here's those test cases which reflect the current behavior in several different contexts.

I also had this one, but I'm convinced this is just a mypy bug. It'd probably be a good regression test there, if this version of TypeOfAny.special_form is ever split to a new type of Any as suggested by python/mypy#15497 (comment).

# Bonus weirdness with TypeAlias
MyType: TypeAlias = Dict[str, Any]
d_type: MyType = {}
special_any = d_type["key"]

assert_type(d_any.get("key", any_value), Any)
assert_type(d_any.get("key", special_any), Any)

assert_type(d_str.get("key", any_value), Any)
assert_type(d_str.get("key", special_any), str)  # ?

This comment has been minimized.

@tungol
Copy link
Contributor Author

tungol commented Dec 10, 2024

For additional context: it's not obvious that all of these variants are needed, but some of them change in non-obvious ways with #13222 so I think it's reasonable.

Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@tungol
Copy link
Contributor Author

tungol commented Dec 11, 2024

Discovered a new type of edge case for dict.get that I wasn't thinking about before: #13229 (comment)

Analysis of some mypy primer results linked, but the theme is what happens if the default value is an empty container type. Right now for dict[str, Any], d_any.get("key", []) returns Any but any change that makes it return Ant | list[Any] opens up a huge amount of noise in mypy primer. It's even worse for nested d_any.get("key1, {}).get("key2", {}). It might be worth adding something in this genre to the tests as well.

Note that for the fully concrete dict[str, str], d_str.get("key", []) already returns str | list[Any] any mypy complains about the unknown container type. For dict[str, list[str]], d_lstr.get("key", []) returns list[str] without issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant