-
Notifications
You must be signed in to change notification settings - Fork 55
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
RFC: begin using python 3 type hints in codebase #3034
Comments
I like this. I would even consider "discovering typing inconsistencies" a pro rather than a con as it reveals to us where things are not functioning as we expect. |
Just ran pytype (which has type inferencing) on the current develop (didn't modify code before running), see output below. It seems like it's catching issues in old code which we should probably remove. Looks like we found a pytype bug (line 166), which would be worked around with an ignore comment... https://gist.github.com/macintoshpie/78a6c811ae8de0d6b41ba0bdd8c4fb57 |
I am for this. I just want to make sure we can lazily apply these updates. I also want to make sure that we focus on the performance work first. |
@nllong yeah I agree, if we "lazily" add type hints on things we touch when working on tickets it should be a pretty good way to get things rolling. I'll have to look into this a bit more, but mypy is probably going to be our better option considering we want more progressive addition of types (rather than having it check our entire codebase for implicit issues). I ran mypy on current develop and got these results, like I said I need to spend a sec looking into why these errors are happening |
TLDR; we decided to progressively include type hints where it's not too much work and could have some payoff. |
Proposal
SEED's python codebase is somewhat complex in some areas. In particular, some functions return semi-complex data structures.
A newer feature of python is using "type hints" to document and validate functions and variables. Having these annotations provide documentation and can be statically checked to verify typing consistency. Types are statically checked (i.e. not at runtime), and can be progressively added to a codebase. See some examples here.
This proposal is for SEED to begin integrating type hints into its python codebase. Below I've listed some of the Pros and Cons.
Pros/Cons
cons
pros
other
types
package, but this project is large enough where it really shouldn't impact performance.UPDATE: see google's notes on type hints as well: https://google.github.io/styleguide/pyguide.html#221-type-annotated-code
UPDATE 2: see Django's DEP for adding type hints here: django/deps#65
TLDR; some concerns about new contributions (b/c of upfront cost of learning/understanding how types work), some concerns about code bloat and chore work such as representing complex types, some praise b/c of confidence refactoring, some praise b/c of much better IDE experience (less code digging)
Integration process
foo
which calls an existing functionbar
the developer should add type hints to both functions.The text was updated successfully, but these errors were encountered: