You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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.)
The text was updated successfully, but these errors were encountered:
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.
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.
Currently, we have two functions
is_subtype
andis_proper_subtype
. There are two corresponding visitors:SubtypeVisitor
andProperSubtypeVisitor
, both of them implement some complex logic.I think that ideally there should be only one visitor,
ProperSubtypeVisitor
, andis_subtype
should be simply: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.)
The text was updated successfully, but these errors were encountered: