Skip to content

Add type hints#70

Draft
Daverball wants to merge 1 commit into
morepath:masterfrom
seantis:feat/typing
Draft

Add type hints#70
Daverball wants to merge 1 commit into
morepath:masterfrom
seantis:feat/typing

Conversation

@Daverball
Copy link
Copy Markdown
Contributor

This is based on the stubs I had already written, but I improved them significantly based on what type checking the tests revealed about the ergonomics.

Some of the type hints are a bit convoluted and difficult to understand, but I don't think we can simplify much without essentially making the type hints useless.

One minor wart these current hints have, is that using the dispatch decorator on a method/classmethod will result in false positives when those methods are invoked on instances of that class. While this is technically solvable to a degree, it will introduce even more protocols and complex overloads and since you're generally supposed to use dispatch_method on methods anyways, to get the proper benefits from the caching lookups, I think it's a reasonable trade-off, even though the test suite verifies that using dispatch works on methods, so I've had to skip type checking for that code-path.

For now I've added tox environments for mypy and pyright. I also very quickly looked at ty and zuban. Neither produced a ton of errors, but it didn't seem worth adding two more sets of pragmas for the few errors they did spit out, especially considering they're both still fairly new and ty doesn't even properly support ParamSpec yet. We can revisit adding support for these more modern rust-based type checkers later.

I would like to add a few dedicated type tests (even though the existing tests already do a good job of exercising the API, it would still be good to put a few assert_type in there, to verify that we're getting the types we think we're getting) and I still need to do a self-review, since using merge-pyi as a starting point almost certainly put some annotations in stupid places, that I missed, so I'll put this in draft for now, but feel free to already take a look at it @henri-hulski and test against your code. I've tested the updated type hints against my code and it helped get rid of a couple of type: ignore comments that have now become redundant, other than that things stayed green, so overall an improvement.

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