Better narrowing with custom equality #20643
Conversation
Co-authored-by: A5rocks
| else_map = {} | ||
| partial_type_maps.append((if_map, else_map)) | ||
|
|
||
| for i in custom_eq_indices: |
There was a problem hiding this comment.
I have a later PR that condenses this a little bit / adds more comments
Maybe worth looking at the version on my dev branch: https://github.com/hauntsaninja/mypy/pull/5/files#diff-f96a2d6138bc6cdf2a07c4d37f6071cc25c1631afc107e277a28d5b59fc0ef04R6699
This comment has been minimized.
This comment has been minimized.
|
graphql one will be fixed by improving logic around this: Lines 8544 to 8546 in 21d6773 I have a commit for this later in my stack Still minimising the dd-trace-py one |
|
Okay dd-trace-py is a "true positive". It gets a little confusing in that |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
A5rocks
left a comment
A5rocks
left a comment
There was a problem hiding this comment.
It's been long enough since my own PR that the logic doesn't make sense at a high level to me... So I'll need to think more to make any comments re: correctness. (given it passes tests, it's obviously correct, though)
| expr_type = operand_types[i] | ||
| expr_type = coerce_to_literal(expr_type) |
There was a problem hiding this comment.
Huh, why did these lines get added? Maybe they're right (not sure what it's doing...) but seems weird to add code to this existing codepath?
There was a problem hiding this comment.
There's no semantic change in the expr_type stuff, the important change here is if i in custom_eq_indices: continue
As for why I have a stray line: managing a stack of like twenty commits is a little fiddly
mypy/checker.py
Outdated
| ...then 'operands' and 'operand_types' would be lists of length 5 and 'chain_indices' | ||
| would be the list [1, 2, 3]. | ||
| The 'narrowable_operand_indices' parameter is the set of all indices we are allowed |
There was a problem hiding this comment.
I think this should say narrowable_indices not narrowable_operand_indices?
There was a problem hiding this comment.
Thanks, fixed!
| # For type_targets, we cannot narrow in the negative case | ||
| # e.g. if (x: str | None) != (y: str), we cannot narrow x to None | ||
| else_map = {} |
There was a problem hiding this comment.
This part of the diff also seems unrelated?
| or_else_maps: list[TypeMap] = [] | ||
| for expr_type in union_expr_type.items: | ||
| if has_custom_eq_checks(expr_type): | ||
| or_if_maps.append({operands[i]: expr_type}) |
There was a problem hiding this comment.
-snip-
Nevermind, I see what this does.
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)
+ tests/execution/test_resolve.py:322: error: Value of type "list[SourceLocation] | None" is not indexable [index]
dd-trace-py (https://github.com/DataDog/dd-trace-py)
+ ddtrace/debugging/_function/discovery.py:219: error: Argument 2 to "undecorated" has incompatible type "str | Any | int | type[staticmethod[Any, Any]] | type[classmethod[Any, Any, Any]]"; expected "str" [arg-type]
|
2 participants
Relates to this comment from A5rocks: #20492 (comment)
Fixes #20532
Fixes #18029
Fixes #17229
Fixes #16465
Closes #18574