Add --doctest-ufunc option to doctest Numpy ufuncs#174
Add --doctest-ufunc option to doctest Numpy ufuncs#174saimn merged 4 commits intoscientific-python:mainfrom
Conversation
lpsinger
force-pushed
the
doctest-ufunc
branch
4 times, most recently
from
1a58f92 to
d2bb21a
Compare
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
This looks very nice to me! Main question is about making numpy a requirement.
tests/ufunc_example/_module2.c
Outdated
| }; | ||
|
|
||
|
|
||
| PyMODINIT_FUNC PyInit__module2(void); /* Silence -Wmissing-prototypes */ |
There was a problem hiding this comment.
What do you mean by 'still'? To the best of my knowledge, this is always necessary to suppress the warning in C if you are declaring a non-static function.
There was a problem hiding this comment.
It's just that I haven't had to do this double declaration (e.g., in pyerfa), and I cannot see any difference in the imports you make...
There was a problem hiding this comment.
It may not be necessary if you don't have warnings cranked all the way up. I'll try without.
lpsinger
force-pushed
the
doctest-ufunc
branch
from
d2bb21a to
2959b1b
Compare
|
The tests are failing due to a warning because I use |
|
pytest 6.2 came out in 2020 Dec, so it's a bit borderlines, but I think it's OK to require it. I'm OTOH would not add numpy as a required dependency, only as an optional. |
|
I am about to bump pytest all the way to 7 at astropy/astropy#12823 😬 |
doctestplus supports much more than astropy core, so if possible I would like to be a bit more conservative here. |
This absorbs the functionality of the pytest-doctest-ufunc package, which was heavily based on pytest-doctestplus to begin with. pytest-doctest-ufunc will be retired. Fixes scientific-python#123.
lpsinger
force-pushed
the
doctest-ufunc
branch
2 times, most recently
from
ced3099 to
151ef0b
Compare
lpsinger
force-pushed
the
doctest-ufunc
branch
from
151ef0b to
5814c0c
Compare
|
Can we just do that by default, without needing a new option ? Is there a downside ? |
Do what by default? |
|
I think @saimn meant whether we can just check the ufunc docstrings by default rather than have a separate option to turn that part on/off. I guess that makes sense - we're really just ensuring here that docstrings that one would naively expect to be checked are in fact checked. |
I'm fine either way. Just say the word. |
|
Hmm, since there doesn't seem a downside, I'd say let's make this automatic! |
|
yes, that's what I meant. I guess the only downside would be the small overhead to check if each function is a ufunc but that should be negligible ? |
The downside is that someone may have exotic code that messes with and wraps functions in a way that is similar to but incompatible with Numpy, and that triggers the code path to search for docstrings in ufuncs, which for one reason or another happens not to work. |
Done. |
|
Have you tried running the p.s. Given pytest 7 and scipy 1.8 introduced a whole bunch of warnings, you might want to wait for astropy/astropy#12823 or make sure you don't use the latest releases from pytest nor scipy. |
Co-authored-by: P. L. Lim <[email protected]>
|
Thank you! |
|
I am going to try this out at astropy/astropy#12853 . Thanks! |
5 participants
This absorbs the functionality of the pytest-doctest-ufunc package, which was heavily based on pytest-doctestplus to begin with.
pytest-doctest-ufunc will be retired.
Fixes #123.