Make PyWavelets an optional dependency#7156
Conversation
denoise_wavelet actually works without PyWavelets for the 1D case if `convert2ycbr` is True and `estimate_sigma` is False. Use xfail instead of pytest.importorskip or pytest.mark.skip, so that the above behavior can actually be tested and we notice if the failure is due something different (not an ImportError).
| >>> import pytest | ||
| >>> _ = pytest.importorskip('pywt') |
There was a problem hiding this comment.
Found this solution here
scikit-image/skimage/feature/_fisher_vector.py
Lines 82 to 83 in cb35369
Would be nice to hide these lines somehow or skip this doctest another way but not sure how.
Not sure if replacing denoise_wavelet with denoise_tv_chambolle in this way makes sense but it prevents having to skip these doctests.
| >>> from skimage.restoration import denoise_tv_chambolle | ||
| >>> import numpy as np | ||
| >>> img = color.rgb2gray(data.astronaut()[:50, :50]) | ||
| >>> rng = np.random.default_rng() | ||
| >>> noisy = img + 0.5 * img.std() * rng.standard_normal(img.shape) | ||
| >>> parameters = {'sigma': np.arange(0.1, 0.4, 0.02)} | ||
| >>> denoising_function = calibrate_denoiser(noisy, denoise_wavelet, | ||
| >>> parameters = {'weight': np.arange(0.01, 0.3, 0.02)} | ||
| >>> denoising_function = calibrate_denoiser(noisy, denoise_tv_chambolle, | ||
| ... denoise_parameters=parameters) |
There was a problem hiding this comment.
Not sure if replacing denoise_wavelet with denoise_tv_chambolle here (and elsewhere) makes sense but it prevents having to skip these doctests. Found an example of denoise_tv_chambolle being used with calibrate_denoiser in this gallery example.
|
Is pywt so pervasive in the restoration submodule that the whole submodule has to raise an importerror? Usually we do it per function, but perhaps there aren't enough other functions around to make it worthwhile. |
Maybe I misunderstand, but the change only raises ImportErrors per function. Though, it's touching more than I originally thought, because |
|
No worries, I was probably seeing https://github.com/scikit-image/scikit-image/pull/7156/files#diff-f676de72662d8ee6bc9afa9e3ca6733b2c6ee1f23a25dbc192f7aef9712d0591R1020 and thought it was top-level. All good. |
Seems like it has to be a pytest.mark in order to work with pytest.param(..., marks=xfail_without_pywt).
for the gallery example plot_cycle_spinning.py.
|
Finally green. CI really keeping me honest here. 😅 Learned some new stuff about pytest's xfail API. |
Description
As discussed in the meeting today, we might want to make PyWavelets an optional dependency.
It is not as lightweight (pure Python) as networkx and currently blocks our release. It is also only used for two of our functions:
denoise_waveletandestimate_sigma.denoise_waveletactually works without PyWavelets for the 1D case ifconvert2ycbrisTrueandestimate_sigmaisFalse.I used
xfailinstead ofpytest.importorskiporpytest.mark.skip, so that the above behavior can actually be tested and we notice if the failure is due something different (not anImportError).I'm marking this as API for lack of a better label. I feel like this would get lost in the maintenance section otherwise. Feel free to change this if you feel like another label fits better.
cc @mkcor, @stefanv, @jarrodmillman
Checklist
./doc/examplesfor new featuresRelease note
Summarize the introduced changes in the code block below in one or a few sentences. The
summary will be included in the next release notes automatically: