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

Unused imports #32

Closed
ixje opened this issue Aug 10, 2020 · 3 comments
Closed

Unused imports #32

ixje opened this issue Aug 10, 2020 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@ixje
Copy link
Contributor

ixje commented Aug 10, 2020

I noticed the following is always included in the generated stubs.

result += [
"from numpy import float64",
"_Shape = Tuple[int, ...]"
]

For example in my case I don't use numpy, so this by default creates invalid stubs. The _Shape doesn't break any, but I also don't need it. Is this a left over or why does it exist? Thanks

@sizmailov
Copy link
Owner

Hi, thanks for report!

This is an ad-hoc fix-up for pybind11-style numpy array annotations which are not supported by other tools due to lack of convention. The line from numpy import float64 is not needed if you using recent version of pybind (newer than pybind/pybind11@22b2504) or not binding functions that use numpy at all.

I agree, those lines should be included only when needed.
Also from numpy import float64 imports most common type while strictly speaking it should be checked which types are really needed. I was lazy back then and included it unconditionally because it "anyway it wont hurt" and I needed it most of time.

Note that imports from typing falls into same "not always necessary" category.

I'll be happy to accept a PR.

@sizmailov sizmailov added bug Something isn't working good first issue Good for newcomers labels Aug 10, 2020
@sizmailov
Copy link
Owner

Hi! I've updated generated stubs to avoid unnecessary imports. It's still imperfect but apparently more correct than it was before.
It's still unconditionally imports typing, probably that's fine.

Feel free to reopen if you see any troubles with new release.

@ixje
Copy link
Contributor Author

ixje commented Aug 20, 2020

Sorry for not replying initially. I've been too bandwidth constrained to even start looking into it. Happy to see you made some improvements 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants