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

Fix stubgen --recursive to only generate stubs for requested submodules. #4845

Merged
merged 3 commits into from Apr 3, 2018

Conversation

Projects
None yet
3 participants
@mgilson
Copy link
Contributor

mgilson commented Apr 3, 2018

This fixes #4844

I'm not completely sure where/how to add unit tests for this. I'm happy to do that if someone can point me in the right direction though.

As it stands, I have tested this manually against urllib and urllib.parse and verified that the current code only generates out/urllib/parse.pyi in the latter case but generates all the stubs for urllib in the former case.

@mgilson mgilson force-pushed the mgilson:stubgen-recursive/4844 branch from 34ae6c6 to cf02970 Apr 3, 2018

path = getattr(package, '__path__', None)
if path is None:
# It's a module inside a package. There's nothing else to walk/yield.
return

This comment has been minimized.

@gvanrossum

gvanrossum Apr 3, 2018

Member

Use continue here, there may be more packages in the packages argument list, we want to process all of them.

This comment has been minimized.

@mgilson

mgilson Apr 3, 2018

Author Contributor

Yep, thanks. Somehow I failed to notice the outer loop.

@gvanrossum

This comment has been minimized.

Copy link
Member

gvanrossum commented Apr 3, 2018

@gvanrossum

This comment has been minimized.

Copy link
Member

gvanrossum commented Apr 3, 2018

This LGTM -- were you going to try and add a test or not? Let me know.

Add tests for mypy.stubgen.walk_packages.
This also adds a place to do unit-testing on other utility functions for the stubgen CLI.

The general idea here is to test `walk_packages` against `mypy` itself since (presumably) is is already imported or will be imported during the tests.  We should also know the exact module structure (unlike something like `urllib` that changes depending on python2.x and python3.x).  I tried not to make the test too prescriptive to reduce the chances that it will need to be updated when new modules or subpackages are added to `mypy`.
@mgilson

This comment has been minimized.

Copy link
Contributor Author

mgilson commented Apr 3, 2018

I've added a test. I don't really know the best way to test something that relies on pkgutil.walk_packages since it imports stuff and that's maybe a little weird. To make it so that we're less likely to change the program state, I'm testing it against mypy itself which at least has some of its submodules already loaded.

Anyway, let me know if you think this is a bad way to go about testing this utility function.

@gvanrossum

This comment has been minimized.

Copy link
Member

gvanrossum commented Apr 3, 2018

This looks like a good start -- thanks for going the extra mile! I'm merging it now.

@gvanrossum gvanrossum merged commit 572dd61 into python:master Apr 3, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.