search

simplistix / sybil

Automated testing for the examples in your documentation.
https://sybil.readthedocs.io/en/latest/
Other
69 stars 14 forks source link

Remove all `func-returns-value` errors from mypy #68

Closed adamtheturtle closed 1 year ago

adamtheturtle commented 1 year ago

This change removes all errors with the code func-returns-value when running mypy --strict sybil tests, and it is one step towards having mypy passing and shipping type hints.

Previous errors looked like:

tests/test_myst_codeblock.py:54: error: "evaluate" of "Example" does not return a value  [func-returns-value]

I have tried to find a balance between checking for the side-effects of evaluate, and not losing the original intentions of confirming that the method returns None. I think that intention is now met by the type checker, but I added two type: ignores for extra safety.

cjw296 commented 1 year ago

To be honest, I'm not worried (or that keen...) to have type annotations on unit tests checked - their execution is a much more valid check of anything...

Type annotations seem valuable on the code that's shipped, should we just exclude the tests folder from consideratiob by mypy instead?

adamtheturtle commented 1 year ago

We can always run mypy only against sybil/ and not against tests/ and then drop this PR and others.

My experience with my own libraries is that it is useful to have a type-hinted consumer of the library which is type checked. It tells me "What is it like to use the library as a type-safe consumer?", and I have found issues like that. For example, if using my library in a type-safe way requires a private import. For my own projects, I find type checking a really good way of finding bugs very quickly, and understanding the causes of bugs. Those bugs are sometimes in the tests, and so I can write tests more quickly.

All of that is a trade-off versus the work and verbosity of type hints. If you choose to close this then I will continue but only on the sybil/ folder.

cjw296 commented 1 year ago

Yeah, let's not run mypy against tests/.

My experience has been the opposite: I have a 100% line coverage baseline for all my open source projects, and I find that catches stuff way before mypy. There's also testing where I want to ensure that run-time passing of the incorrect type is handled correctly, which adds annoyance.

cjw296 commented 1 year ago

Interesting question: is the stuff indocs/examples currently checked? I think it probably should be, since those are user-facing examples.

adamtheturtle commented 1 year ago

Interesting question: is the stuff indocs/examples currently checked? I think it probably should be, since those are user-facing examples.

I have not been checking those, but it can be done by running mypy docs/.