Handle NumPy 1.14 API changes#2935
Conversation
|
There seems to be a lot of issues with the recent numpy release besides the ones fixed via this PR: These are mostly due to the new pretty printing of ndarrays. |
|
As a stop gap measure one can use |
|
Nah, I think that is a half measure. @scikit-image/packaging do you foresee any issues if we update the docstrings in accordance with the new standard? Is |
|
@jarrodmillman What was your solution for this in NetworkX? |
|
@soupault what do you mean? skimage gets tested with minimal requirements in our CI... The real solution would be to fix doctest to detect equivalences, but that seems improbable. I personally do favour printing in legacy mode until we no longer depend on NumPy <1.14. |
|
@jni ah, indeed, we have one
Would it imply adding a config line to each of the mentioned docstrings? |
|
FYI, here is what we did for nx: |
|
@jarrodmillman 2818 is super depressing LOL. @soupault I really hope that there is an option in pytest to run specific code before each doctest. i don't know if that's true. If not then let's just give up on life. =P Because we have a lot more docstrings depending on numpy than NX does. |
|
@oleksandr-pavlyk I think there are several more to address. For example, |
…ith np.issubdtype(dt, np.floating)
There was a problem hiding this comment.
This PR looks good.
There are couple of warning related to np.issubdtype (see https://travis-ci.org/scikit-image/scikit-image/jobs/329168966), but they will be addressed within #2888.
|
It is not clear whether match.py#L57 needs replacing It was probably meant as An instance where turning on the deprecation warning likely caught a genuine bug. |
|
Just in case you didn't find it, you can apply the legacy printing package-wide with this in your It will only apply to the pytest run because pytest imports |
|
@oleksandr-pavlyk it seems that it does not pass for all Travis configurations... |
I'm going to finalize the solution within this PR
…_), since the former is interpreted as issubdtype(dt, np.generic) per deprecation warning, clearly unintended here
… per deperecation warning
|
Two travis jobs failed with unrelated issues, and 4 passed. XCode build is pending. |
|
|
| # List of files that pytest should ignore | ||
| # Use legacy numpy printing. This fix is made to keep doctests functional. | ||
| # For more info, see https://github.com/scikit-image/scikit-image/pull/2935 . | ||
| # TODO: remove this workaround once minimal required numpy is set to 1.14.0 |
There was a problem hiding this comment.
Please add this to TODO.txt, then we're ready to go.
Codecov Report
@@ Coverage Diff @@
## master #2935 +/- ##
=========================================
Coverage ? 86.03%
=========================================
Files ? 337
Lines ? 27050
Branches ? 0
=========================================
Hits ? 23273
Misses ? 3777
Partials ? 0
Continue to review full report at Codecov.
|
|
@Borda tried twice (including Travis cache removal). There seems to be some issues with the pre-release versions of the dependencies. |
|
@soupault do you mean you suggest to merge anyway, despite the problems with Travis? |
|
@emmanuelle yes, that's correct. |
|
I do not feel happy about merging without full pas on Travis, but on the other hand, it is much better to have this mostly fix then nothing... shall we also create a new PR for the rest of required changes? |
|
@Borda what remains to be done? (in a separate PR) |
|
It seems there is an Shinx issue |
|
How do I find out what is different between invoking the build with no set environment, and with I created /pull/2947, hoping to see if the phenomenon is specific to changes made in this commit, but errors this PR resolves muddle the picture. Suggestions? |
|
@oleksandr-pavlyk With |
|
Thank you @soupault . Inspecting outputs of
Here is the full list. |
|
The change in Sphinx 1.7 removed support for configuration variable I was only able to find a commented out reference in |
|
In Sphinx 1.7 sources, a plugin for The issue needs to be fixed there. Going to raise an issue. This PR should be merged please. The sooner the better. |
|
In it goes; thanks! |
Borda
left a comment
There was a problem hiding this comment.
@oleksandr-pavlyk @stefanv @soupault
I found an issue in this correction such that any version lover that 1.10 pass throw the condition and crash for unsupported legacy parameter... rather use this
# parse the numpy versions
np_version = [int(i) for i in np.version.full_version.split('.')]
# comparing strings does not work for version lower 1.10
if np_version >= [1, 14]:
np.set_printoptions(legacy='1.13')
| # TODO: remove this workaround once minimal required numpy is set to 1.14.0 | ||
| import numpy as np | ||
|
|
||
| if np.version.full_version >= '1.14.0': |
There was a problem hiding this comment.
fails for version lower than 1.10
|
although I see requirements that says minimal 1.11 :) |
Yes, that was exactly my thinking. :) |
* fixed NumPy 1.14 deprecation warning * replaced remaining instances of deprecated np.issubdtype(dt, float) with np.issubdtype(dt, np.floating) * repacing issubdtype(dt, np.bool) with intended issubdtype(dt, np.bool_), since the former is interpreted as issubdtype(dt, np.generic) per deprecation warning, clearly unintended here * issubdtype(dt, 'half') replaced with issubdtype(dt, np.half) per deprecation warning * replaced issubdtype(dt, np.int) with issubdtype(dt, np.signedinteger) per deperecation warning * Attempt at replacing use of np.subtract with use of np.logical_xor for arrays of boolean type per NumPy deprecation warning * Use legacy numpy printing in pytest * fixed incorrect replacement of np.issubdtype(dt, 'half') with np.issubdtype(dt, np.half). NumPy 1.13 was interperting that as np.issubdtype(dt, np.floating) * Use view to convert arrays of type np.bool into np.uint8 when passing them to scipy.ndimage functions to work around deprecation warnings in NumPy 1.14 on arithmetic with np.bool arrays * Added reminedr on pytest doctest workaround to TODO
A number of test errors result when testing scikit-image with just released NumPy 1.14. The following change fixes the following errors:
The fix is the same as was applied to scikit-learn.