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

is_subtype vs is_proper_subtype #3297

Closed
ilevkivskyi opened this issue May 1, 2017 · 1 comment · Fixed by #13303
Closed

is_subtype vs is_proper_subtype #3297

ilevkivskyi opened this issue May 1, 2017 · 1 comment · Fixed by #13303
Labels
refactoring Changing mypy's internals

Comments

@ilevkivskyi
Copy link
Member

ilevkivskyi commented May 1, 2017

Currently, we have two functions is_subtype and is_proper_subtype. There are two corresponding visitors: SubtypeVisitor and ProperSubtypeVisitor, both of them implement some complex logic.
I think that ideally there should be only one visitor, ProperSubtypeVisitor, and is_subtype should be simply:

def is_subtype(left, right):
    if isinstance(left, AnyType) or isinstance(right, AnyType):
        return True
    return is_proper_subtype(left, right)

Are there any reasons why it is not possible now? Maybe this question already exists in a roadmap mentioned in #3194 (btw @JukkaL @ddfisher is it possible to make it public?)

(I have found this issue #2038, but I am not sure it is up to date.)

@ilevkivskyi ilevkivskyi added the refactoring Changing mypy's internals label May 1, 2017
@JukkaL
Copy link
Collaborator

JukkaL commented May 2, 2017

It's more complex than that, since List[str] is considered a subtype of List[Any], etc. Maybe we could have a flag for SubtypeVisitor that indicates whether we should be taking a proper or normal subtype. I thought about merging the two when writing the proper subtype visitor, but it didn't seem obvious at that time that combining would make the code easier to read or maintain. However, it might well be worth it, since they duplicate a lot of logic.

ilevkivskyi added a commit that referenced this issue Aug 3, 2022
Fixes #3297

This removes a significant chunk of code duplication. This is not a pure refactor, there were some cases when one of the visitors (mostly non-proper one) was more correct and/or complete. In few corner cases, where it was hard to decide, I merged behavior with `if` checks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Changing mypy's internals
Projects
None yet
2 participants