-
Notifications
You must be signed in to change notification settings - Fork 504
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
Roadmap to strict
typing with Mypy
#6279
Comments
There are some tools which may help to add annotations to old code. See https://mypy.readthedocs.io/en/stable/existing_code.html#automate-annotation-of-legacy-code. In particular I have tried autotyping and it's pretty easy/straightforward to use. There is also mypy_clean_slate, which can fast-forward the use of |
IMO the biggest hurdle here is not the mypy execution options, i.e. I suspect a large number of errors are related to vtk version differences. The only way I know how to fix this is to declare one version of the upstream library suitable for typing and then slap |
I'm not trying to argue against some of the proposals here, but I'd focus first on getting the dependencies aligned with typing here and only then adding in more mypy options. Otherwise, the number of errors when including vtk in the mypy dependencies will continue to grow. Edit: I missed this sentence in your description " I believe these should be addressed first before enabling more options." My apologies. You already have this covered. |
@user27182, type hints have been on my "would love to have" list for a while since I have a few downstream projects that need it. If you figure out the approach and want me to work on a module/directory, let me know. It's a bit annoying to have my dependent projects warn about |
This might be a slightly separate issue, but very related. Because of our inheritance choices, it is extremely difficult to type the filters correctly. This is because the filters require the When I have tried in the past, without having the inheritance sorted, it requires slapping a |
@akaszynski sounds good, we'll let you know. I think we're still at the "figure out the approach" stage.
This will be important to resolve. The exact mechanism for solving this (e.g. protocols vs. inheritance) can probably be discussed in #4774. For now, we'll just need to continue |
I have been able to isolate 219 errors attributed exclusively to including VTK as a dependency. See #6740 for a list of errors. Indeed there are many errors in |
I have found an approach that seems to work: use autotyping to automate adding annotations on a per-file basis, then resolve any outstanding type errors that are reported after the changes. I have found that this is relatively straightforward if you restrict the number of "auto-typed" files such that there are fewer than 10-15 type errors to resolve. See #6802 and #6803. Once the "obvious" cases are auto-typed, more serious type errors that were previously ignored can be addressed. @akaszynski I am willing to add type hints and fix some of the existing type errors but this will need support in terms of reviews and consensus from maintainers/contributors. E.g. I think #6795 needs to be approved before a serious effort to add type hints can be made. If I push typing-related PRs, should I request a review from you? |
Yes please, tough lately to get time for PR reviews, but it's high priority and I'll just need to be reminded. |
Describe what maintenance you would like added.
Type annotations make code easier to read and help prevent bugs. Having 100% annotations would also make it easy to switch to using
autodoc-typehints
(see #5665) and simplify the documentation. To this extent, I propose working towards using the--strict
option withmypy
. This issue can be used to track progress towards this goal.This section in the Mypy docs shows how this can be worked up to by enabling some options one at a time. The options for these are shown in "Add config options" below.
In addition there are some other outstanding issues with running
mypy
CI checks for the project. I believe these should be addressed first before enabling more options.Fix outstanding issues
mypy
environment for CI checks (we currently have two configs, one inpyproject.toml
and another inpre-commit-config.yaml
), see #6257 (commet)mypy
imports project files correctly (e.g. resolve pre-commitmypy
config is not catching errors reliably #5611)mypy pyvista
reports 144 type errors which are not caught bypre-commit run mypy
#5563Add config options
Start off with these
warn_unused_configs = True
Addwarn_unused_configs
tomypy
config #6792warn_redundant_casts = True
Addwarn_redundant_casts
to mypy config #6124warn_unused_ignores = True
Add--warn-unused-ignores
tomypy
config #5589Getting these passing should be easy
strict_equality = True
Addstrict_equality
tomypy
config #6793strict_concatenate = True
Addextra_checks
tomypy
config #6794Strongly recommend enabling this one as soon as you can
check_untyped_defs = True
Addcheck_untyped_defs
tomypy
config #6795These shouldn't be too much additional work, but may be tricky to get passing if you use a lot of untyped libraries
disallow_subclassing_any = True
disallow_untyped_decorators = True
disallow_any_generics = True
Add--disallow-any-generics
tomypy
config #5593These next few are various gradations of forcing use of type annotations
disallow_untyped_calls = True
disallow_incomplete_defs = True
disallow_untyped_defs = True
This one isn't too hard to get passing, but return on investment is lower
no_implicit_reexport = True
This one can be tricky to get passing if you use a lot of untyped libraries
warn_return_any = True
Links to source code.
No response
Pseudocode or Screenshots
No response
The text was updated successfully, but these errors were encountered: