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

Make isinstance more powerful #220

Merged
merged 26 commits into from
Jul 5, 2023
Merged

Make isinstance more powerful #220

merged 26 commits into from
Jul 5, 2023

Conversation

nielstron
Copy link
Contributor

@nielstron nielstron commented Jul 3, 2023

This implements a number of improvements for isinstance handling

  • isinstance is also a type cast for if expressions, boolean expressions, loops and asserts i.e
    • x.bar if isinstance(x, A) else ... now works
    • isinstance(x, A) and x.bar now works (shortcut expression, left and ensures type is valid in the right part)
    • isinstance(x, A) or x.foo now works (shortcut expression, left or ensures type is invalid in the right part)
    • while isinstance(x, A): print(x.bar) now works
    • assert isinstance(x, A); print(x.bar) now works
    • if isinstance(x, A): ... else: x.foo now works where x is cast to inverted in the else branch
  • isinstance now works in conjunction with other terms as long as typecasting is guaranteed i.e. isinstance(x, A) and isinstance(y, B) and x.foo == y.bar now works
  • isinstance(x, A) or isinstance(x, B) now casts x to Union[A, B]. note that isinstance(x, A) or isinstance(x, B) or whatever does not cast to anything since no type is actually guaranteed.
  • not isinstance(x, A) now casts Union[A, B, ...] to Union[B, ...]
  • the combination of and, or and not work in arbitrary and recursive combination

Note: This introduces a breaking change

While the below worked previously, this will break from this change onwards. The reason is that x is casted to "whatever union does not contain A" implicitly in the else branch and this can not be overwritten. Pulling in opinions on whether this will break real-life usecases from @juliusfrost @chrissiwaffler

if isinstance(x, A):
  ...
else:
  x = A(foo)

@juliusfrost
Copy link
Contributor

I tested this in https://github.com/OpShin/opshin-pioneer-program with no issues

@chrissiwaffler
Copy link
Contributor

No, I also don't see conflicts in any of my code.

Appreciate the changes! @nielstron
This will make things a lot easier with typecasting :)

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.

3 participants