This starts with some tweaks to the docstring data in tools (.. versionadded:: kinda stuff) and tools.calculation that I found sitting in a forgotten branch; apparently those smaller tweaks never reached the point of a full PR.
That stalled work's finally done: sopel/tools/calculation.py is now fully type-annotated and has 100% test coverage. Along the way I discovered a few API glitches (e.g. incorrectly documented parameter types, missing parameters) and fixed 'em.
I admit some of the test assertions are pretty ugly, but with only one custom exception type it's necessary to make sure that the right kind of ExpressionEvaluator.Error is being raised. A future refactoring opportunity, that is (which #2508 already tracks).
Just one thing I haven't cracked: Two new Sphinx warnings from type annotations that it can't resolve to cross-reference links:
/home/dgw/github/sopel-irc/sopel/sopel/tools/calculation.py:docstring of
sopel.tools.calculation.ExpressionEvaluator:1: WARNING: py:class reference
target not found: ast.operator
/home/dgw/github/sopel-irc/sopel/sopel/tools/calculation.py:docstring of
sopel.tools.calculation.ExpressionEvaluator:1: WARNING: py:class reference
target not found: ast.unaryop
Those are, however, the types required to satisfy mypy. Trying to use the CamelCase classes in ast's documentation (BinOp and UnaryOp) never passed type-checking. The class hierarchy there doesn't work like one might think it does (or should).
At the moment, I'm ready to give up on these like what eventually happened with the unfixed warnings left over in #2496. Really seems like Sphinx just can't handle certain things. Even the Python docs themselves reference these, and the rendered HTML shows them as xref py py-class but they're not linked.
Description
This starts with some tweaks to the docstring data in
tools(.. versionadded::kinda stuff) andtools.calculationthat I found sitting in a forgotten branch; apparently those smaller tweaks never reached the point of a full PR.That stalled work's finally done:
sopel/tools/calculation.pyis now fully type-annotated and has 100% test coverage. Along the way I discovered a few API glitches (e.g. incorrectly documented parameter types, missing parameters) and fixed 'em.I admit some of the test assertions are pretty ugly, but with only one custom exception type it's necessary to make sure that the right kind of
ExpressionEvaluator.Erroris being raised. A future refactoring opportunity, that is (which #2508 already tracks).Checklist
make qa(runsmake lintandmake test)Notes
The framework for fully exercising
tools.calculationin tests came from both #2530 and https://github.com/sopel-irc/sopel/pull/2464#issuecomment-1676170000 — thanks, @SnoopJ! Getting around to this would have taken a lot longer if you didn't kick-start it.Just one thing I haven't cracked: Two new Sphinx warnings from type annotations that it can't resolve to cross-reference links:
Those are, however, the types required to satisfy
mypy. Trying to use the CamelCase classes inast's documentation (BinOpandUnaryOp) never passed type-checking. The class hierarchy there doesn't work like one might think it does (or should).At the moment, I'm ready to give up on these like what eventually happened with the unfixed warnings left over in #2496. Really seems like Sphinx just can't handle certain things. Even the Python docs themselves reference these, and the rendered HTML shows them as
xref py py-classbut they're not linked.