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

feat: typing support for helpers #2588

Merged
merged 4 commits into from
Oct 14, 2020
Merged

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Oct 13, 2020

Description

The goal here is to enable external typing support, so users can type check code containing pybind11 calls (just the import pybind11, not something magical with compiled extensions)! It is also checked internally with mypy, as well, integrated into the pre-commit checks.

However, it did expose a small logical issue when checked internally; sysconfig.get_path logically is not required to return a string, so we could be adding None's to the path. Fixed by only adding truthy strings.

If this is too large to go into 2.6.0, we can go with #2589 instead.

Suggested changelog entry:

- Added external typing support to the helper module, code from ``import pybind11`` can now be type checked.

henryiii added a commit to henryiii/pybind11 that referenced this pull request Oct 13, 2020
@henryiii henryiii changed the title feat: basic typing support for helpers feat: typing support for helpers Oct 14, 2020
Copy link
Collaborator

@YannickJadoul YannickJadoul left a comment

Choose a reason for hiding this comment

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

As far as I'm concerned this is not too big or too extensive to still add to 2.6.0. But neither would I expect it's very urgent (do people actually add typing to their setup.py?).

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 14, 2020

It doesn't really touch real code (save for the one mentioned change), just adds supporting files mostly, so I rather think it's fine too.

do people actually add typing to their setup.py?

Given how poor support for typing is in typeshed for distutils (build_ext was missing), I wouldn't expect so. But that could/should change? And, besides, many people use PyCharm (myself included once in a while), and this will just start helping them work with our tools immediately. :)

neither would I expect it's very urgent

Nope, which is why I didn't get around to it until now. (Plus it's best to add a pyi file after the py file is mostly stable).

@YannickJadoul
Copy link
Collaborator

And, besides, many people use PyCharm (myself included once in a while), and this will just start helping them work with our tools immediately. :)

Ah yes, if it helps, why not. This one CI job keeps the .pyi in sync with the actual code, right? Will it complain if there's a new function added, for example?

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 14, 2020

It should complain if you add a function that has no type info (the --strict parameter). If you lie about types(*), this is also caught by the internal type checking (external entities only see the .pyi when type checking code against us, if there is one with the same name). The reason to use .pyi over type comments is to avoid importing typing and added a dependency pre-3.5; if it was possible to not add a .pyi, I did avoid it.

*: Lying defined as making a type statement that does not satisfy the code type check. You may lie about inner details, inheritance, skip non public methods, etc, and that's not lying in this sense.

Copy link
Collaborator

@rwgk rwgk 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 include in 2.6.0.

pybind11/setup_helpers.pyi Show resolved Hide resolved
@henryiii henryiii added this to the v2.6.0 milestone Oct 14, 2020
@github-actions github-actions bot added the docs Docs or GitHub info label Oct 14, 2020
@henryiii henryiii merged commit 645d838 into pybind:master Oct 14, 2020
@henryiii henryiii deleted the feat/types branch October 14, 2020 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Docs or GitHub info
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants