Add annotation stubs for `PyGraph` and `PyDiGraph`
Related to #352
This PR adds type annotation stubs, it contains .pyi files with type annotations. I started by annotating the PyGraph and PyDiGraph files, as well as the custom types.
I tried to follow the PEP 561 convetion: we include .pyi files in the same directory as our Python code. Note that this is a first step in annotations: they are partial, we can add more as we go
We also test the annotations using pytest-mypy-testing. I had to add a new Tox environment to keep them separate from the unittests we have. The tests are under the tests/stubs directory.
Some items we need to handle because the PR has been open for long:
- [ ] Add
PyGraphandPyDiGraphmethods that were added after the PR was created - [ ] Add custom return types that were added after the PR was created
Pull Request Test Coverage Report for Build 2778787088
- 0 of 0 changed or added relevant lines in 0 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.008%) to 97.14%
| Totals | |
|---|---|
| Change from base Build 2778191284: | 0.008% |
| Covered Lines: | 12499 |
| Relevant Lines: | 12867 |
💛 - Coveralls
I need to verify the newly added functions to PyGraph and PyDiGraph, and also check if there were updates to function signaturs. But this PR is ready to review
I've no power on retworkx - can someone please assign me to this so I don't forget to check it?
I'm generally in favour of libraries providing type hints if at all possible - it's hopefully not too much work, and lets downstream users make the choice over whether or not to use them.
I think there's a couple of problems with the typing here, though, some of which seem to stem from design decisions in
PyDiGraphandPyGraph. The edge data is inherently nullable, but the type hints here don't account for that. This means that the types are inconsistent, and all graph type hints should effectively bePyGraph[S, Union[T, None]]. You don't want users to do that, so anyPyGraphfunction that returns typeTshould instead returnUnion[T, None]. Obviously this means that it'll be harder for users to write compliant code, but that's actually a function of the internal typing ofPyGraph, and technically it's required whether or not the type hint says that. (See also: why everyone hates nullable types!)
I am back on this and I think the solution is along the lines of restricting the methods you outlined.
So basically what we want is to never allows users to do this:
graph: PyGraph[str, int] = PyGraph()
node_a: int = graph.add_node("A")
node_b: int = graph.add_node("B")
graph.add_edges_from_no_data([(node_a, node_b)]) # mypy catches this error
But let users do this:
graph: PyGraph[str, Optional[int]] = PyGraph()
node_a: int = graph.add_node("A")
node_b: int = graph.add_node("B")
graph.add_edges_from_no_data([(node_a, node_b)]) # mypy should let this pass
That way we do not need to make users handle None unless they chose to
I rewrote the custom return types annotations from zero. Because they are generated from the same Rust macro, I created an abstract class representing each macro and inherited from them. It shortens the annotations from that section considerably.
I also made the types covariant to handle some issues pointed earlier.
Pull Request Test Coverage Report for Build 4378374086
- 0 of 0 changed or added relevant lines in 0 files are covered.
- 1 unchanged line in 1 file lost coverage.
- Overall coverage increased (+0.03%) to 97.145%
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| src/shortest_path/all_pairs_dijkstra.rs | 1 | 99.27% |
| <!-- | Total: | 1 |
| Totals | |
|---|---|
| Change from base Build 4376765708: | 0.03% |
| Covered Lines: | 14053 |
| Relevant Lines: | 14466 |
💛 - Coveralls
Alright, this is ready for review after so long. mypy.stubtest checks that the stubs in this PR are identical to the text_signature from our Rust code and that the return types are valid.
Some things might be missing (e.g. all functions from rustworkx.rustworkx), but this is a good start to support annotations