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

Better names of and more compatibility between ad hoc intersections of instances #18506

Merged
merged 5 commits into from
Jan 22, 2025

Conversation

tyralla
Copy link
Collaborator

@tyralla tyralla commented Jan 22, 2025

While working on #18433, we encountered this bug.. @ilevkivskyi identified the underlying problem, and we decided to try to reuse previously created ad hoc intersections of instances instead of always creating new ones.

While working on this PR, I realised that reusing intersections requires more complete names. Currently, module and type variable specifications are not included, which could result in mistakes when using these names as identifiers.

So, I switched to more complete names. Now, for example, <subclass of "A" and "A"> becomes <subclass of "mod1.A[builtins.int]" and "mod2.A">. Hence, I had to adjust many existing test cases where reveal_type is used.

testReuseIntersectionForRepeatedIsinstanceCalls confirms that the mentioned bug is fixed.

testIsInstanceAdHocIntersectionIncrementalNestedClass and testIsInstanceAdHocIntersectionIncrementalUnions are in a separate commit. I think they are not really necessary, so we might prefer to remove them. I added them originally because I had to adjust lookup_fully_qualified a little. The change is very simple, but I could not create a test case where it is not sufficient.

@tyralla tyralla requested a review from ilevkivskyi January 22, 2025 06:00
@tyralla tyralla changed the title Better names of more compatibility between ad hoc intersections of instances Better names of and more compatibility between ad hoc intersections of instances Jan 22, 2025
@tyralla tyralla changed the title Better names of and more compatibility between ad hoc intersections of instances Better names and of more compatibility between ad hoc intersections of instances Jan 22, 2025
@tyralla tyralla changed the title Better names and of more compatibility between ad hoc intersections of instances Better names of and more compatibility between ad hoc intersections of instances Jan 22, 2025

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pyodide (https://github.com/pyodide/pyodide)
- src/py/pyodide/_state.py:21: error: Incompatible types in assignment (expression has type "pyodide._state.<subclass of "ModuleType" and "JsProxy">1", target has type "pyodide._state.<subclass of "ModuleType" and "JsProxy">")  [assignment]
- src/py/pyodide/_state.py:21: error: Incompatible types in assignment (expression has type "pyodide._state.<subclass of "ModuleType" and "JsProxy">2", target has type "pyodide._state.<subclass of "ModuleType" and "JsProxy">")  [assignment]

@ilevkivskyi
Copy link
Member

Thanks!

@ilevkivskyi ilevkivskyi merged commit 878d892 into python:master Jan 22, 2025
18 checks passed
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.

2 participants