-
Notifications
You must be signed in to change notification settings - Fork 49
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
Type annotate the public API #167
Conversation
This makes it easier for tools to provide completion and enhances the documentation of the API.
This makes it behave better with newer ruff versions, and removes the unused isort configuration.
This enables the use of newer Python typing functionality without compatibility shims.
This ensures that the documentation build uses a well-defined Python version, rather than whatever happens to be the default for the environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should have py.typed
marker file inside the pyproject_hooks
package.
Thanks @pradyunsg - it looks like @layday has already done a detailed review of the annotations, so I'll skip doing that. My inclination is to be fairly conservative with Python version support, since this is meant to be a boring piece of low-level infrastructure that doesn't change often. I see pip hasn't dropped 3.7 support yet, so I'd suggest we don't here either. But I'm not going to argue about this if you really want to. |
This reverts commit 69ed8b1.
This information is now coming from the source code directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've type-checked this branch against the main branch of build; there was only one error, which looks like something that should be fixed on build's side:
src/build/__init__.py:180: error: Argument "runner" to "BuildBackendHookCaller" has incompatible type "Callable[[Sequence[str], Optional[str], Optional[Mapping[str, str]]], None]"; expected "Optional[SubprocessRunner]" [arg-type]
(This is presumably because the protocol has default arguments.)
This makes it possible to annotate using this type, instead of relying on redefining it.
The latest commit makes it possible to do...
And then annotate subprocess runners using that type. |
Instead of leaving that as a comment, I've gone ahead and documented that. |
Wouldn’t it make sense to allow |
That's what we do with the |
On the basis of "this is better than status quo", I'm gonna go ahead and merge this. There's a couple of potential follow ups to improve this further: |
(I'll fix the documentation build separately) |
Closes #90