cleo
cleo copied to clipboard
feat(typing): added `py.typed` marker file
Resolves: #74
This PR waits until src/cleo/ui/table module is properly typed.
While the table module is not typed for now, I think this can be applied anyway, since it's only one module (see #265 ).
First, even ignoring cleo.ui.table, cleo is not currently clean:
src/cleo/ui/exception_trace.py:270: error: Argument 2 to "_render_exception" of "ExceptionTrace" has incompatible type
"Optional[BaseException]"; expected "Exception" [arg-type]
self._render_exception(io, inspector.previous_exception)
^
Found 1 error in 1 file (checked 71 source files)
(ideally this wants fixing in crashtest I think, this should be a BaseException here and the crashtest.inspector.Inspector should deal in BaseException)
As for cleo.ui.table: it may only be one module, but if the types are bad then that's an imposition on all users of this library. IMO it's bad form to publish types that are unhelpful, which seems to be what this is doing at the moment.
Less bad from a consumer point of view - though still annoying! - would be to remove types from those functions whose types you can't figure out how to fix. That's at least close to being explicit about "we haven't typed this function" rather than publishing types that you know are wrong.
First, even ignoring
cleo.ui.table, cleo is not currently clean:src/cleo/ui/exception_trace.py:270: error: Argument 2 to "_render_exception" of "ExceptionTrace" has incompatible type "Optional[BaseException]"; expected "Exception" [arg-type] self._render_exception(io, inspector.previous_exception) ^ Found 1 error in 1 file (checked 71 source files)(ideally this wants fixing in
crashtestI think, this should be aBaseExceptionhere and thecrashtest.inspector.Inspectorshould deal inBaseException)As for
cleo.ui.table: it may only be one module, but if the types are bad then that's an imposition on all users of this library. IMO it's bad form to publish types that are unhelpful, which seems to be what this is doing at the moment.Less bad from a consumer point of view - though still annoying! - would be to remove types from those functions whose types you can't figure out how to fix. That's at least close to being explicit about "we haven't typed this function" rather than publishing types that you know are wrong.
Thanks for your insight, I will look into this type error. As to incomplete typing, I have decided to postpone pushing this and finish the typing game.
@dimbleby what option did you activate to see this error? mypy doesn't show this
mypy doesn't show this
well of course it does, I didn't make it up!
At a guess the problem is that you - and the CI pipeline - are only running mypy through precommit. This is inadequate because the pre-commit environment does not contain all the other libraries and type stubs that mypy needs to do proper analysis.
You probably want to shift mypy checking to being a 'proper' step in the CI workflow, cf what poetry does
well of course it does, I didn't make it up!
I wasn't trying to say that you made this up, I simply didn't know how to reproduce this. Shame that mypy doesn't work well with pre-commit. I will change the setup.
Closing in favor of #274