MAINT: Use contextmanager in _run_doctests #15430
Conversation
There was a problem hiding this comment.
Side question.
This docstring results in a warning intentionally being printed
https://github.com/numpy/numpy/blob/master/numpy/core/_ufunc_config.py#L105
Do you know of a pattern to expect a warning in the doctest?
I could execute the docstring line in an with assert_warns but that seemed overkill.
| @@ -25,21 +25,23 @@ | |||
|
|
|||
| $ python refguide_check.py --rst docs | |||
| """ | |||
| import sys | |||
There was a problem hiding this comment.
I can revert the sorting of imports if alphabetical isn't the way to sort them.
|
|
||
| return success, output | ||
| return success, output.read() |
There was a problem hiding this comment.
output is now a str instead of array but this method starts with _ so I think this is fine.
sethtroisi
force-pushed
the
refguide_warning
branch
from
243797d
to
8cb531c
Compare
sethtroisi
force-pushed
the
refguide_warning
branch
from
8cb531c
to
a546d6a
Compare
|
@rgommers expressed some concerns about refguide cleanups - sorry for not mentioning that earlier. It might be ok, so long as you make the same fixes upstream in scipy. |
sethtroisi
force-pushed
the
refguide_warning
branch
from
a546d6a
to
a71e8a8
Compare
I'll try to upstream this (either after this is merged or given LGTM) with the caveat that I've never submitted to scipy so the outcome is uncertain. |
charris
changed the title
|
I think our version has diverged significantly from scipy since we have the doc/rst checks from gh-14732 which I do not think were merged to scipy. |
|
@rgommers Will you have some time to review this PR this week? I'm eager to get it committed so I can try to upstream it and other changes to scipy. |
There was a problem hiding this comment.
This looks like a reasonable cleanup. I haven't testing, but it looks fine to merge. Thanks @sethtroisi
|
Thanks for taking a look Ralf! |
|
This broke error reporting. Before this would show the actual doctest failures. Now it only reports that the test fails, without any output. If we cannot fix it quickly, I will have to revert the merge. |
|
I'm still awake and will take a look but if you don't here from me in ~30 minutes please revert. |
No description provided.