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

Add type annotations via MonkeyType #469

Merged
merged 12 commits into from Oct 25, 2019
Merged

Conversation

bhrutledge
Copy link
Contributor

@bhrutledge bhrutledge commented Jun 16, 2019

Following @carljm's comments on #231 and #466, and watching his PyCon 2018 talk, I decided to back up a bit, and experiment with using MonkeyType to generate the annotations based on the test suite.

$ source .tox/py37/bin/activate
$ pip install monkeytype mypy
$ monkeytype run -m pytest tests
$ monkeytype apply twine.repository
$ git commit -am "Apply monkeytype to twine.repository"

There was some non-trivial clean-up (e.g., overly-specific Unions, replacing package: stub with package: PackageFile, etc.), but overall I found it to be straightforward and educational re: type hinting in general and the Twine codebase.

Also, mypy is passing at the moment, so I stopped ignoring the result in testenv:lint.

As before, I'm curious what folks think.

@bhrutledge
Copy link
Contributor Author

This should be rebased/redone (and maybe closed and resubmitted) once #498 (Twine 2.0) is merged.

@bhrutledge bhrutledge self-assigned this Oct 5, 2019
@pypa pypa deleted a comment from codecov bot Oct 25, 2019
@pypa pypa deleted a comment from codecov bot Oct 25, 2019
@bhrutledge bhrutledge added the incomplete Work in progress label Oct 25, 2019
@bhrutledge bhrutledge changed the title WIP: mypy via monketype mypy via monketype Oct 25, 2019
@bhrutledge bhrutledge changed the title mypy via monketype Add type annotations via MonkeyType Oct 25, 2019
@bhrutledge
Copy link
Contributor Author

After rebasing on master, this is now passing, with initial annotations for twine.package and twine.repository. I'm planning to add more type coverage, though I'm not sure how complete it can/should be before this is merged.

Copy link
Sponsor Member

@di di left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine to merge as-is!

@sigmavirus24 sigmavirus24 merged commit f43ef0a into pypa:master Oct 25, 2019
@bhrutledge bhrutledge deleted the monkeytype branch October 25, 2019 16:46
@bhrutledge
Copy link
Contributor Author

Thanks!

@bhrutledge
Copy link
Contributor Author

Also, thanks @waseem18 for your initial work on this!

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

Successfully merging this pull request may close these issues.

None yet

4 participants