Skip to content

Make stubgen less aggressive about walking C extension modules #5900

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

Merged
merged 2 commits into from
Nov 16, 2018

Conversation

msullivan
Copy link
Collaborator

PR #5814 made stubgen try to walk as a module any attribute of a C
of a C extension module of module type. This is too aggressive: mypyc
generated extension modules will often have attributes of module type
from imports that do not correspond to actual modules.

This causes a stubgen test to fail under mypy_mypyc while trying to
import mypy.errors.os, since mypy.errors imports os.

Only walk such a submodule when its __name__ matches it being a
submodule.

PR #5814 made stubgen try to walk as a module any attribute of a C
extension module of module type. This is too aggressive: mypyc
generated extension modules will often have attributes of module type
from imports that do not correspond to actual modules.

This causes a stubgen test to fail under mypy_mypyc while trying to
import `mypy.errors.os`, since `mypy.errors` imports `os`.

Only walk such a submodule when its `__name__` matches it being a
submodule.
@msullivan msullivan requested a review from gvanrossum November 16, 2018 01:00
@gvanrossum
Copy link
Member

Before I approve this, a question for @pkerichang: Can you test this with your pybind11 setup? (This could go awry if pybind11 doesn't set __name__ to the fully qualified module name.)

@msullivan
Copy link
Collaborator Author

I'm going to go ahead and merge this now, so that mypyc tests get fixed. @pkerichang, if this breaks what you were trying to do in #5814, follow up with us

@msullivan msullivan merged commit f737d37 into master Nov 16, 2018
@msullivan msullivan deleted the stubgen-mypyc branch November 16, 2018 20:21
@pkerichang
Copy link
Contributor

Sorry for the late response; this works fine on my end. Thanks for letting me know.

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.

3 participants