scikit-image / skimage-tutorials

skimage-tutorials: a collection of tutorials for the scikit-image package.
Other
493 stars 374 forks source link

remove skimage viewer in tutorials #55

Open sciunto opened 3 years ago

sciunto commented 3 years ago

I propose to remove scripts using the skimage viewer.

pep8speaks commented 3 years ago

Hello @sciunto! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 5:1: E265 block comment should start with '# ' Line 6:1: E265 block comment should start with '# ' Line 7:1: E265 block comment should start with '# ' Line 51:1: E265 block comment should start with '# ' Line 53:1: E265 block comment should start with '# ' Line 54:1: E265 block comment should start with '# ' Line 54:80: E501 line too long (80 > 79 characters) Line 55:1: E265 block comment should start with '# ' Line 55:80: E501 line too long (83 > 79 characters) Line 56:1: E265 block comment should start with '# ' Line 56:80: E501 line too long (83 > 79 characters) Line 57:1: E265 block comment should start with '# ' Line 57:80: E501 line too long (81 > 79 characters) Line 58:1: E265 block comment should start with '# ' Line 58:80: E501 line too long (86 > 79 characters) Line 59:1: E265 block comment should start with '# ' Line 60:1: E265 block comment should start with '# '

jni commented 3 years ago

@sciunto I agree with @rfezzani's suggestion. This is what git history is for. =)

stefanv commented 3 years ago

Am I missing something? This change seems to remove the exploration examples without putting something in its place. Perhaps replace using Napari? Jupyter widgets?

grlee77 commented 3 years ago

Am I missing something? This change seems to remove the exploration examples without putting something in its place. Perhaps replace using Napari? Jupyter widgets?

For clock_deblur.py, I don't think there is any point in keeping the example at all without the viewer component because no debluring is being performed and there is no output to the example.

It doesn't seem high priority to merge this PR until we are ready to remove the viewer module itself. Ideally we could replace with something based on Napari or Dash as you suggest, but that requires someone familiar with those packages spending the time to do it! @jni is Napari at a state where something like the interactive clock_deblur.py excample would be reasonably easy to implement?

jni commented 3 years ago

is Napari at a state where something like the interactive clock_deblur.py excample would be reasonably easy to implement?

Yes, it would be ~trivial. Here's a relevant example in the magicgui docs. The only thing I don't think we have yet is triggering computation on release instead of on slide. @tlambert03 does it make sense to add such a feature request for magicgui?

tlambert03 commented 3 years ago

does it make sense to add such a feature request for magicgui?

Yeah for sure

tlambert03 commented 3 years ago

added to magicgui in https://github.com/napari/magicgui/pull/248, here's an example that mostly follows the clock deblur example using napari & magicgui:

edited to include descriptive text example

import numpy as np
from skimage.data import clock
import napari
from magicgui import magicgui, widgets

image = clock()
M, N = image.shape

# Should pad, but doesn't make much difference in this case
MM, NN = 2 * M + 1, 2 * N + 1

# Apply Hann window to prevent ringing
wy = np.hanning(M)[:, None]
wx = np.hanning(N)

f = np.zeros((MM, NN))
f[:M, :N] = wy * wx * image

F = np.fft.fft2(f)

v, u = np.ogrid[:MM, :NN]
v -= (MM - 1) // 2
u -= (NN - 1) // 2

@magicgui(
    auto_call=True,
    T={"widget_type": "FloatSlider", "max": 1, "tracking": False},
    a={"widget_type": "FloatSlider", "min": -0.1, "max": 0.1, "tracking": False},
    b={"widget_type": "FloatSlider", "min": -0.1, "max": 0.1, "tracking": False},
    K={"widget_type": "FloatSlider", "min": 0.1, "max": 100, "tracking": False},
    clip={"widget_type": "FloatSlider", "max": 1000, "tracking": False},
)
def apply_inverse_filter(T=0.5, a=0, b=0, K=15, clip=750) -> "napari.types.ImageData":
    uavb = u * a + v * b
    H = T * np.sinc(uavb) * np.exp(-1j * np.pi * uavb)
    H = np.fft.fftshift(H)

    HH = 1.0 / H
    HH[np.abs(HH) > K] = K

    gg = np.abs(np.fft.ifft2(F * HH))
    gg = gg[:M, :N]
    gg = np.clip(gg, 0, clip)
    gg -= gg.min()
    gg /= gg.max()
    return gg

viewer = napari.view_image(image)

info = """
<h2>Inverse Filter</h2>
Welcome to the inverse filter tutorial!<br>
Here's a description of some of the<br>
parameters...

<dl>
    <dt>T</dt>
    <dd>....</dd>

    <dt>a</dt>
    <dd>....</dd>
</dl>
"""
apply_inverse_filter.insert(0, widgets.Label(value=info))
viewer.window.add_dock_widget(apply_inverse_filter, name='Inverse Filter')

napari.run()

(side-notes: setting tracking False didn't seem to be critical here... it's reasonably snappy without it. also, it seemed like the image parameter was unused in the previous apply_inverse_filter example... so might want to double check my version)

stefanv commented 3 years ago

Very nice, @tlambert03! It would be good to have a way to point the user's attention to the added widget (otherwise it blends in well with the rest of Napari). And also to add a title to the widget set.

But this is a great proof of concept showing that we can go ahead and port our examples over.

tlambert03 commented 3 years ago

Agreed! I updated the example to show how you can provide a title to the widget set, and include a label (accepts html) to draw attention, looks like this:

Untitled

(note also: magicgui will parse docstrings and provide tooltips based on the parameter docstrings)

stefanv commented 3 years ago

@tlambert03 That is perfect; thank you!