-
Notifications
You must be signed in to change notification settings - Fork 55
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 native python type annotations #73
Comments
#74 is an example of an error that might've been mitigated/avoided with explicit type hints & a typechecker such as mypy/pyright. |
on a related note, it might also be nice to use a type-checking/validation library such as on the other hand, some of the changes i'm proposing are maybe breaking changes; e.g., this package is currently marked as supporting python3.3, but adding type annotations is probably going to raise the version requirement to at least 3.7, if not 3.8 or later. on the other hand python3.8 and earlier are already marked end of life. so i'm sort of trying to ask @hfaran, what are you prioritizing with this package? do you want to maximize stability & backwards-compatibility, or is having some more modern features & more robust type-safety, perhaps introducing some breaking changes & at the expense of some backwards-compatbility, acceptable? would it be better for me to work on some PRs or try to make a fork for more "aggressive" changes? |
I have no interest in supporting EOL'd versions of python. The minimum supported python version can be updated to 3.9. So the more "aggressive" changes are fine so long as we minimize breaking changes to the existing API of this package. |
types of arguments are already documented in the docstrings as
:type param: ...
. by now python has had built-in support for type annotations via PEP484 for many years (since at least 3.7, if not earlier), and the features are relatively stable/mature.we should use them so we can take advantage of things like IDE type hint tooling, etc., and also screen out other errors in the existing piazza-api codebase!
The text was updated successfully, but these errors were encountered: