typeddicts - update conformance tests according to the spec #1978
Conversation
| # > A key that is not a literal should generally be rejected. | ||
| movie: Movie = {variable_key: "", "year": 1900} # E: variable key | ||
|
|
||
| # Destructive operations. | ||
| existing_movie[variable_key] = 1982 # E: variable key |
There was a problem hiding this comment.
I'd like to get clarification on the wording in the spec before we add these tests. I've started a discussion here. Feel free to add your viewpoint to the thread.
|
|
||
| # It's not clear from the spec what type this should be. | ||
| movie.get("name") | ||
|
|
||
| # It's not clear from the spec what type this should be. | ||
| movie.get("other") # E? | ||
|
|
||
| # It's not clear from the spec whether it's allowed. | ||
| reveal_type("other" in movie) # E? |
There was a problem hiding this comment.
In general, we should not add tests to the conformance suite in areas where the spec is not clear. If you think that clarity is needed (and is beneficial), then please start a discussion about amending the spec.
There was a problem hiding this comment.
I think I've overlooked the spec a bit - I've made assumption that in with unknown key is ambigious similar to movie.get("other") # E? but now I've found that spec actually states that all operations with unknown key should be rejected:
The use of a key that is not known to exist should be reported as an error, even if this wouldn’t necessarily generate a runtime type error.
So I assume it's the same situation as with arbitrary strings - spec prohibits it, but in reality
- setitem, getitem, del - rejected by everyone (mypy, Pyre, pyright)
.pop("other")- rejected by Pyre and mypy.get("other")- only rejected by Pyre"other" in movieis not rejected by any type checker
Can you please take a look if I'm not missing something? And I'll start a similar discussion.
There was a problem hiding this comment.
The spec doesn't provide any specific guidance for how type checkers should synthesize methods on TypedDicts (get, pop, etc.). Unless you think there's value in standardizing this behavior, we should leave the spec and the conformance tests as is. If you think there's value in standardizing the behavior here (I'm somewhat skeptical that it's worth the effort), then we should amend the spec and update the conformance tests accordingly.
get should never be rejected because it's safe to call (i.e. doesn't raise an exception) for a not-present key. It even has a form that accepts a default value that is returned when the key isn't present.
The in operator should also never be rejected because it never raises an exception.
My preference is to leave the conformance test as it is currently and abandon this PR. I think the current test reflects what is in the spec.
Andrej730
changed the title
Added missing tests to typeddicts_operations.py based on the current state of https://typing.python.org/en/latest/spec/typeddict.html#supported-and-unsupported-operations
Stumbled upon type checkers having a different behaviour for
typed_dict[str], investigated the spec and the spec is clear about type checkers generally rejecting operations with arbitrary strings - so wanted to reflect the spec in conformance tests.Added tests for:
popitem.getand within- spec is not certain about this, so I guess it allows both behaviours..getandin.Added baseline output commit just for a reference - to confirm that there no other typecheckers version changes at play in the updated output. I can remove it before merge to avoid additional diffs.