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

provide a type rewriter to find common base class #298

Closed
dgutson opened this issue Aug 4, 2023 · 5 comments
Closed

provide a type rewriter to find common base class #298

dgutson opened this issue Aug 4, 2023 · 5 comments

Comments

@dgutson
Copy link

dgutson commented Aug 4, 2023

Let's assume we have

file1.py: defines MyEnum1 that extends Enum.
file2.py: likewise MyEnum2
file3.py: defines function F (untyped)

file1 calls F passing an element of MyEnum1, file2 likewise with MyEnum2.

MonkeyType will type F's argument as a Union of MyEnum1 and MyEnum2.
This is not only invalid because it would require a cyclic import (since file3 would need to import file1 and file2 to get the enum definitions), but also the Union explodes when F is a library and is being called from many places.

My proposal is that MonkeyType will find a common base class that can be imported without incurring in a cyclic import.

@carljm
Copy link
Contributor

carljm commented Aug 8, 2023

Thanks for the suggestion!

This is a case that falls under MonkeyType's clearly documented minimalist philosophy: it aims to give you accurate and concrete information about the types seen in your program, and let you make the decisions about how to best abstract those into good annotations.

If we added this feature, it would lead to judgment calls that would probably end up requiring new config options. E.g. what happens if there are twenty different subclasses of a common base type and only two of them are ever seen at a particular location? Is the base type still the best annotation, or is the union better?

Or what about cases where the base type is very broad, e.g. some kind of base "object" type in a large type hierarchy? We could special-case the Python object type, but users can create similar type hierarchies with similar common bases (e.g. if writing a type-checker in Python!), and may not want all unions reduced to Object.

Fundamentally this transformation of the annotation loses potentially useful information, and since we can't be sure it is preferred in all cases, it's better not to do the transformation by default.

This is why MonkeyType has "type rewriters" which allow arbitrary post-transformation of types. The transformation you want can be written as a type rewriter and used in your own projects. I'd even consider merging it into MonkeyType so it's available to all users (though I don't think I would have it be turned on by default.)

@carljm
Copy link
Contributor

carljm commented Aug 8, 2023

I'm leaving the issue open in case someone wants to provide a PR implementing this as an optional but built-in type rewriter.

@carljm carljm changed the title Find common base class provide a type rewriter to find common base class Aug 8, 2023
@dgutson
Copy link
Author

dgutson commented Aug 8, 2023

@arieltorti @qequ

@carljm
Copy link
Contributor

carljm commented Dec 21, 2023

Fixed with merge of #311

@carljm carljm closed this as completed Dec 21, 2023
@dgutson
Copy link
Author

dgutson commented Dec 22, 2023

thanks @chrhansk !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants