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

WIP: mypy support #466

Closed
wants to merge 9 commits into from
Closed

WIP: mypy support #466

wants to merge 9 commits into from

Conversation

bhrutledge
Copy link
Contributor

Related to #231, building on #344.

I'm a newbie to mypy, but at @brainwane's suggestion, I decided to dive in as an exercise. I've skimmed the Zulip blog (which is somewhat outdated, e.g. CLI options and broken links), read mypy's getting started and existing codebase docs, and reviewed the existing PR's.

I got lint-mypy to pass in 8f569a5 (covering some of the ground from #359), then added check_untyped_defs as suggested by Zulip. I know the preference is to use stub files, but the initial batch of errors seemed to require local variable annotations. I'd love some feedback on this direction.

Current errors:

twine/wheel.py:56: error: Item "None" of "Optional[Match[str]]" has no attribute "group"
twine/utils.py:216: error: Module has no attribute "get_credential"
twine/utils.py:241: error: Module has no attribute "get_password"
tests/test_check.py:27: error: "Callable[[str], int]" has no attribute "calls"
tests/test_check.py:38: error: "Callable[[str], int]" has no attribute "calls"

@brainwane
Copy link
Contributor

@carljm is there some kind of "hey mypy experts! help?" bat-signal I can shine? If so, help me shine it? :-)

@carljm
Copy link

carljm commented Jun 11, 2019

@brainwane I commented on #231 with some thoughts about overall approach to typing in twine. As for bat signal, a lot of Python typing experts hang out in https://gitter.im/python/typing -- you could ask specific questions there for sure, you could also post a link to this PR and see if anyone is interested in reviewing it.

@codecov
Copy link

codecov bot commented Jun 15, 2019

Codecov Report

Merging #466 into master will decrease coverage by 0.19%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #466     +/-   ##
=========================================
- Coverage   82.93%   82.73%   -0.2%     
=========================================
  Files          14       14             
  Lines         750      753      +3     
  Branches      108      108             
=========================================
+ Hits          622      623      +1     
- Misses         92       94      +2     
  Partials       36       36
Impacted Files Coverage Δ
twine/utils.py 82.39% <100%> (-1.3%) ⬇️
twine/repository.py 73.58% <100%> (+0.25%) ⬆️
twine/commands/__init__.py 100% <100%> (ø) ⬆️
twine/package.py 86.84% <80%> (+0.11%) ⬆️
twine/wininst.py 31.57% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update add0aea...989b401. Read the comment docs.

@bhrutledge
Copy link
Contributor Author

Note: there's an alternate approach in #469, leveraging MonkeyType.

@bhrutledge
Copy link
Contributor Author

Closing in favor of #469, assuming #437 will be merged.

@bhrutledge bhrutledge closed this Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants