Run doctests in pytest. Add linters to pre-commit. Add a pre-commit workflow.#96
Conversation
a48ca40 to
03245c8
Compare
nclack
left a comment
There was a problem hiding this comment.
Looks good to me.
I know you've got a question for @andy-sweet and I wonder about other things to get into the pre commit (e.g. ruff and cargo fmt). So feel free to hold off on merging till you're comfortable. But I think this can go in as is.
There was a problem hiding this comment.
Running doctests as part of pre-commit feels a bit heavy to me (e.g. compared to linting), especially when those tests depend on locally connected hardware.
I think that pytest should just pick up on any doctests in Python modules by default and we can modify the glob pattern to also look at the Markdown files (and Rust files if needed). In that case, we don't need to add any new workflows either (though adding a remote pre-commit might be a good idea).
|
I added a check on the .pyi files to test_docs.py and removed the pre-commit check, opting to just call |
andy-sweet
left a comment
There was a problem hiding this comment.
Mostly looks good, but I don't think we need to explicitly collect doctests with test_docs.py, in which case we can and should make a few simplifications.
…te pytest args in workflow files.
andy-sweet
left a comment
There was a problem hiding this comment.
This PR is mostly about adding code format checkers to pre-commit (and the resulting changes that causes) than it is about running the tests on documented examples.
I'm fine with that (i.e. no need to split the PR), but at least update the title and description to match that.
| exclude = [ | ||
| "__init__.py" | ||
| ] | ||
|
|
||
| ignore = [ | ||
| "F403", | ||
| "F405", | ||
| ] No newline at end of file |
There was a problem hiding this comment.
I'm OK to start with these excludes and ignores, though I'd prefer to have none of them in the long term.
Uh oh!
There was an error while loading. Please reload this page.