Finalize type signatures and enforce for all future functions #1061
Conversation
This commit adds the last few missing type hints (primarily the generators module) and switches the CI configuration over to enforce that the library is completely type hinted. This makes it a CI error if a function or method is added that doesn't include type hints. This is all built on the hard work of @IvanIsCoding who built up all the type hinting infrastructure and stub files for the majority of rustworkx. This is just the last piece to enforce the library is fully typed moving forward. Co-authored-by: Ivan Carvalho <[email protected]>
Pull Request Test Coverage Report for Build 7601942772
💛 - Coveralls |
mtreinish
changed the title
This commit fixes most of the package for stubtest to work. This primarily involves having the stubfiles match the package layout. Unfortunately this means we need to put the majority of the type hints in the stubfile for `rustworkx.rustworkx`. Having the stubfiles split up as before mypy complains that the module doesn't exist. For example, having the coloring functions in `rustworkx/coloring.pyi` causes a mypy error that `rustworkx.coloring` couldn't be imported. This causes the rustworkx.pyi file to be quite large, but I couldn't find a pattern to workaround this limitation. Aside from that a bunch of stubs needed to be updated as now that they're getting included in the stubtest run issues were caught. The one exception is the generators module where the custom `text_signature` set in the function definitions were incorrect and tripping up mypy, these were fixed by removing the `text_signature` fields.
There was a problem hiding this comment.
Thanks for wrapping this up! Just some thoughts before merging:
- The re-exports are awful but mypy cannot find type-specific functions #960 was a real reported issue and mypy does not re-export symbols. None of our users import from
rustworkx.rustworkxso I guess we need the duplication at the root - Maybe
from __future__ import annotationsfixes the errors in 3.8? - We should add a release note to celebrate 😃 🍾
| label: str | ||
| connectionstyle: str | ||
|
|
||
| def mpl_draw( |
There was a problem hiding this comment.
This is really awesome! I personally would have annotate just with Any because even matplotlib still lacks annotation suport nowadays
Do you think we can get away from a |
We need to re-export with the |
This commit adds back the re-exports to the root `__init__.pyi` stub file. As was reported in Qiskit#960 the manual re-exports are necessary to ensure that mypy can find symbols in the root of the package, where most people use them, instead of off the inner `rustworkx.rustworkx` module.
|
Since Qiskit#1061 has merged rustworkx is now fully typed with mypy. This means that adding type annotations is no longer optional and is now required for all new function. This commit updates the contribution guidelines to indicate type annotations are required now. The details are also updated a bit to provide a bit more detail on what's required when adding type annotations to rustworkx.
Since #1061 has merged rustworkx is now fully typed with mypy. This means that adding type annotations is no longer optional and is now required for all new function. This commit updates the contribution guidelines to indicate type annotations are required now. The details are also updated a bit to provide a bit more detail on what's required when adding type annotations to rustworkx. Co-authored-by: Ivan Carvalho <[email protected]>
This commit adds the last few missing type hints (primarily the generators module) and switches the CI configuration over to enforce that the library is completely type hinted. This makes it a CI error if a function or method is added that doesn't include type hints. This is all built on the hard work of @IvanIsCoding who built up all the type hinting infrastructure and stub files for the majority of rustworkx. This is just the last piece to enforce the library is fully typed moving forward.
TODO:
.to_dot()signature so it doesn't error and thatgraphviz_draw()'s use of it passes mypy