Repository navigation
Move the join-based narrowing logic towards a union-based narrowing logic. - #18212
tyralla wants to merge 9 commits into
Conversation
Sorry, something went wrong.
…ogic. More concretely: use `make_simplified_union` instead of `join_simple` in `ConditionalTypeBinder.update_from_options`
for more information, see https://pre-commit.ci
|
Just a draft, I did not find the button to mark it accordingly... |
Sorry, something went wrong.
This comment has been minimized.
…ing_towards_union_based_narrowing' into narrowing/move_join_based_narrowing_towards_union_based_narrowing
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: pydantic (https://github.com/pydantic/pydantic)
+ pydantic/v1/main.py:901: error: Set comprehension has incompatible type Set[int | str]; expected Set[str | None] [misc]
+ pydantic/v1/main.py:901: note: Left operand is of type "set[str] | dict_keys[str, Any]"
pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/computation/expr.py:680: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/computation/expr.py:680: error: Item "Attribute" of "Attribute | Name" has no attribute "id" [union-attr]
+ pandas/core/computation/expr.py:680: note: Error code "union-attr" not covered by "type: ignore" comment
+ pandas/core/computation/expr.py:702: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/computation/expr.py:702: error: Item "Attribute" of "Attribute | Name" has no attribute "id" [union-attr]
+ pandas/core/computation/expr.py:702: note: Error code "union-attr" not covered by "type: ignore" comment
mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ pymongo/synchronous/cursor.py:1002: error: Invalid index type "int | Any | Pattern[Any]" for "list[Any]"; expected type "SupportsIndex" [index]
+ pymongo/asynchronous/cursor.py:1004: error: Invalid index type "int | Any | Pattern[Any]" for "list[Any]"; expected type "SupportsIndex" [index]
|
Sorry, something went wrong.
ilevkivskyi
left a comment
There was a problem hiding this comment.
I don't like this approach, it is way too ad hoc. A more principled solution would be to not use joins at all in update_from_options(). But this will likely cause a bigger fallout, and will also require adjusting some edge cases.
This would be more consistent with ternary expressions, and IIRC this is the last place join_simple() is used. Ideally join/meet should be only used by the constraint solver.
Sorry, something went wrong.
Btw I just checked and there are very few left (apart from the one being discussed), couple ad-hoc uses in checkexpr.py, and few more in checkpattern.py. |
Sorry, something went wrong.
|
If "move towards..." is not "principled" enough, I better close this PR. I could try a complete step later (and learn a little about the edge cases of narrowing), but please do not hesitate to do it yourself if you have the time and interest (or if you expect less work when implementing than when reviewing it). |
Sorry, something went wrong.
Fixes #16379
Only a modest change. PR #18180 brought me to issue #18238. Trying to fix the latter revealed inconsistencies between join-style and union-style narrowing, so I decided to deal with this issue first. And here is the (maybe too simple?) solution...
Regarding the primer output: For pandas, a (possibly debatable) false negative turns into a similar one. The affected pydantic and pymungo functions are complicated and not stringently typed. I would say the new error messages are okay, but I am not 100% sure.