Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove skimage viewer in tutorials #55

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

remove skimage viewer in tutorials #55

wants to merge 1 commit into from

Conversation

sciunto
Copy link
Member

@sciunto sciunto commented Mar 31, 2021

I propose to remove scripts using the skimage viewer.

@pep8speaks
Copy link

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 '# '

Copy link
Member

@rfezzani rfezzani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thank you @sciunto

Comment on lines +5 to +7
#from skimage.viewer import ImageViewer
#from skimage.viewer.widgets import Slider
#from skimage.viewer.plugins.base import Plugin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#from skimage.viewer import ImageViewer
#from skimage.viewer.widgets import Slider
#from skimage.viewer.plugins.base import Plugin

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hesitated to remove being afraid that the logic would be difficult to understand afterwards. No strong feelings through.

Comment on lines +51 to +60
#viewer = ImageViewer(image)
#
#plugin = Plugin(image_filter=apply_inverse_filter)
#plugin += Slider('T', 0, 1, value=0.5, value_type='float', update_on='release')
#plugin += Slider('a', -0.1, 0.1, value=0, value_type='float', update_on='release')
#plugin += Slider('b', -0.1, 0.1, value=0, value_type='float', update_on='release')
#plugin += Slider('K', 0, 100, value=15, value_type='float', update_on='release')
#plugin += Slider('clip', 0, 1000, value=750, value_type='float', update_on='release')
#viewer += plugin
#viewer.show()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#viewer = ImageViewer(image)
#
#plugin = Plugin(image_filter=apply_inverse_filter)
#plugin += Slider('T', 0, 1, value=0.5, value_type='float', update_on='release')
#plugin += Slider('a', -0.1, 0.1, value=0, value_type='float', update_on='release')
#plugin += Slider('b', -0.1, 0.1, value=0, value_type='float', update_on='release')
#plugin += Slider('K', 0, 100, value=15, value_type='float', update_on='release')
#plugin += Slider('clip', 0, 1000, value=750, value_type='float', update_on='release')
#viewer += plugin
#viewer.show()

@jni
Copy link
Member

jni commented Apr 23, 2021

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

@stefanv
Copy link
Member

stefanv commented May 5, 2021

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
Copy link

grlee77 commented May 5, 2021

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
Copy link
Member

jni commented May 12, 2021

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
Copy link

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

Yeah for sure

@tlambert03
Copy link

tlambert03 commented Jul 10, 2021

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
Copy link
Member

stefanv commented Jul 12, 2021

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
Copy link

tlambert03 commented Jul 12, 2021

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
Copy link
Member

stefanv commented Jul 12, 2021

@tlambert03 That is perfect; thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants