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

Add baseline for types #143

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Add baseline for types #143

merged 4 commits into from
Oct 11, 2024

Conversation

sarayourfriend
Copy link
Collaborator

This PR adds mypy and baseline type-checking infrastructure to CI and the project's scripts.

For this PR, type checking is restricted to the interceptors, and follow-up PRs can add annotations and checking to other files individually. I've done this to prevent the size of this PR from getting out of hand.

When adding annotations to a previously unannotated file, we'll add that file (or directory, if possible) to the types script's list of explicit places to check. After we've annotated all of src/pook, we can condense the list down to just that. Finally, we'll enable type checking for the tests.

Ideally, the addition of annotations does not require introducing runtime code changes. At times, though, it seems like it will be, if the types are to be useful for Pook itself (rather than just on the surface for users). It's certainly better if the project itself has correct and useful annotations to rely on, rather than just API-surface level stubs. The reason for that are:

  1. So that maintainers of pook get to benefit from the hints and help that type annotations add throughout the entire code. I've already seen some potentially incorrect code just from doing basic exploratory type annotating in the project. Those aren't in this PR, but will come up later, and would help the project maintain internal integrity.
  2. So that we do not need to maintain separate type stubs which may go out of sync with the actual API surface of the module.

Moving the Request::url setter was required to work around a strange mypy bug where it did not think the property was settable if the setter was not defined immediately after the original property declaration
@sarayourfriend sarayourfriend merged commit 2d7c699 into master Oct 11, 2024
7 checks passed
@sarayourfriend sarayourfriend deleted the add/some-types branch October 11, 2024 05:06
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.

1 participant