8000 Use CI to make sure # doctest: +ELLIPSIS is not in docs · Issue #17019 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Use CI to make sure # doctest: +ELLIPSIS is not in docs #17019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
thomasjpfan opened this issue Apr 23, 2020 · 5 comments · Fixed by #17269
Closed

Use CI to make sure # doctest: +ELLIPSIS is not in docs #17019

thomasjpfan opened this issue Apr 23, 2020 · 5 comments · Fixed by #17269

Comments

@thomasjpfan
Copy link
Member

Following from #16992
@GaelVaroquaux
Copy link
Member

cc @lucyleeow if you have spare cycles

@lucyleeow
Copy link
Member

AFAICT there is no option to do this with flake8. You could do it with a flake8 plugin (e.g., https://github.com/i02sopop/flake8-banned-words) but we'd need to rely on an external plugin.

I could also use a grep function (e.g., grep --include=\*.py -rnw sklearn/ -e '# doctest: +ELLIPSIS'), capture the exit code and raise an error if pattern found.

@cmarmo
Copy link
Contributor
cmarmo commented May 12, 2020

@thomasjpfan @jnothman I'm quite sure a PR has just been merged removing a forgotten "# doctest: +NORMALIZE_WHITESPACE", but I can't find it right now. Anyway, finding this one (#13991) helped me to better understand what is the goal of this issue.
Shouldn't be more suitable to check for custom doctest directive in the CI, not only ELLIPSIS?

@thomasjpfan
Copy link
Member Author

I could also use a grep function (e.g., grep --include=*.py -rnw sklearn/ -e '# doctest: +ELLIPSIS'), capture the exit code and raise an error if pattern found.

We most likely need to check the rst as well.

Shouldn't be more suitable to check for custom doctest directive in the CI, not only ELLIPSIS?

I agree we should check for "# doctest: +NORMALIZE_WHITESPACE". I think a greping solution as suggested by @lucyleeow would work for our use case.

@lucyleeow lucyleeow mentioned this issue May 18, 2020
Merged
@rth
Copy link
Member
rth commented May 18, 2020

Ideally it would have been nice to run this as part of pre-commit checks as well, so that these checks could be easily run locally, but so far I have not found an obvious way of doing that. There is a number of setups that forbid something https://pre-commit.com/hooks.html but not one that takes as input a list of regexp...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Check default doctest directives in CI lucyleeow/scikit-learn
5 participants
@GaelVaroquaux @rth @cmarmo @thomasjpfan @lucyleeow
0