DOC: update to scipy-doctest 1.6.0 and fix tests #28023
Conversation
mattip
commented
Dec 17, 2024
mattip
commented
- update scipy-doctest to 1.6.0
- remove the check-docs from the linux benchmark CI run, if desired this should become a separate run
- fix failing doctests
| ['??->?', 'bb->b', 'BB->B', 'hh->h', 'HH->H', 'ii->i', 'II->I', 'll->l', | ||
| 'LL->L', 'qq->q', 'QQ->Q', 'ff->f', 'dd->d', 'gg->g', 'FF->F', 'DD->D', | ||
| 'GG->G', 'OO->O'] | ||
| ['??->?', 'bb->b', 'BB->B', 'hh->h', 'HH->H', 'ii->i', 'II->I', ... |
There was a problem hiding this comment.
Doctest does not like splitting lines, and I think a few examples is plenty. BTW, the docstring itself (on lines 5069-5074) is pretty unclear. I think the type signature mini-language is documented elsewhere, perhaps this docstring should point there?
There was a problem hiding this comment.
Not sure why long lines are an issue, but I agree that this can be short and this seems fine.
Not sure we have a good place to link to (maybe the ufunc registration), but the few who have a reason to look at it, know about it anyway, I think...
numpy/polynomial/_polybase.py
Outdated
| @@ -685,9 +685,11 @@ def degree(self): | |||
|
|
|||
| Create a polynomial object for ``1 + 7*x + 4*x**2``: | |||
|
|
|||
| # set the print format to ascii for doctests | |||
| >>> np.polynomial.set_default_printstyle("ascii") | |||
There was a problem hiding this comment.
I could have used 'unicode' but I am not sure what windows CI does...
There was a problem hiding this comment.
For this one, I think it would be nice to keep the unicode, because users are more likely to see it then not, I believe.
I suspect numpy/conftest.py is read? Maybe we can do p.polynomial.set_default_printstyle("unicode") there to remove windows from the equation?
(I think different defaults is just about bad terminals, which might print oddly if the test fails, but shouldn't influence test results.)
But if this isn't trivial, we can also just do this.
There was a problem hiding this comment.
I changed it to unicode and checked that it works on my windows 10 machine. I prefer to leave the np.polynomial.set_default_printstyle in the example so it is more obvious there is an alternative. MAybe @rossbar knows why windows defaults to ascii?
There was a problem hiding this comment.
OK, maybe @rossbar can follow up if he prefers a different approach, would be nice to get this in.
IIRC, some windows terminals (older powershell?) have poor unicode printing support (it may even have just been the font missing symbols).
The overview docs and the docs of set_default_printstyle itself mention it, so I think that setting it globally would be OK, but then those two places should make sure to reset it to whatever it (with a comment that docs use unicode).
|
Closes #28019 |
| ['??->?', 'bb->b', 'BB->B', 'hh->h', 'HH->H', 'ii->i', 'II->I', 'll->l', | ||
| 'LL->L', 'qq->q', 'QQ->Q', 'ff->f', 'dd->d', 'gg->g', 'FF->F', 'DD->D', | ||
| 'GG->G', 'OO->O'] | ||
| ['??->?', 'bb->b', 'BB->B', 'hh->h', 'HH->H', 'ii->i', 'II->I', ... |
There was a problem hiding this comment.
Not sure why long lines are an issue, but I agree that this can be short and this seems fine.
Not sure we have a good place to link to (maybe the ufunc registration), but the few who have a reason to look at it, know about it anyway, I think...
numpy/polynomial/_polybase.py
Outdated
| @@ -685,9 +685,11 @@ def degree(self): | |||
|
|
|||
| Create a polynomial object for ``1 + 7*x + 4*x**2``: | |||
|
|
|||
| # set the print format to ascii for doctests | |||
| >>> np.polynomial.set_default_printstyle("ascii") | |||
There was a problem hiding this comment.
For this one, I think it would be nice to keep the unicode, because users are more likely to see it then not, I believe.
I suspect numpy/conftest.py is read? Maybe we can do p.polynomial.set_default_printstyle("unicode") there to remove windows from the equation?
(I think different defaults is just about bad terminals, which might print oddly if the test fails, but shouldn't influence test results.)
But if this isn't trivial, we can also just do this.
… azp][skip cirrus]
| @@ -6131,13 +6122,12 @@ | |||
| >>> (arr + arr).dtype.metadata | |||
| mappingproxy({'key': 'value'}) | |||
|
|
|||
| But if the arrays have different dtype metadata, the metadata may be | |||
| dropped: | |||
| If the arrays have different dtype metadata, the first one wins: | |||
There was a problem hiding this comment.
N.B.: The previous example says "currently", so I think this is fine, but we shouldn't make promises around this.
(the only true safe thing to do, IMO is to never propagate unless the dtype has custom logic)
FWIW, this works for our numerical dtypes, one already can't expect it to work e.g. with strings.
There was a problem hiding this comment.
I am not sure about the current rules, but the test was failing so I fixed the test and the comment accordingly.
