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

Add example of creating tools using HoloViz Panel and HoloViews #6033

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

MarcSkovMadsen
Copy link

@MarcSkovMadsen MarcSkovMadsen commented Nov 13, 2021

Description

Adds an example of creating tools using HoloViz Panel and Holoviews requested in #6023

sobel-algo-with-save-speedup.mp4

Todo

  • Add Panel and HoloViews to dependencies
  • Fix pep8 issues
  • Find a way to build and test docs when on Windows
  • Test that example works

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

@pep8speaks
Copy link

pep8speaks commented Nov 13, 2021

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

Line 15:80: E501 line too long (113 > 79 characters)
Line 22:70: W291 trailing whitespace
Line 30:80: E501 line too long (120 > 79 characters)

Comment last updated at 2022-02-19 15:27:12 UTC

@MarcSkovMadsen
Copy link
Author

MarcSkovMadsen commented Nov 13, 2021

I'm not able to install latex on my constrained, windows laptop and thus cannot build the docs and test them.

I've tried using binder without being able to get a development environment up and running.

If somebody knows a working, easy way to get a dev/ test environment up and running please let me know. Thanks.

@mkcor
Copy link
Member

mkcor commented Nov 16, 2021

Dear @MarcSkovMadsen,

Thank you for this great contribution. I have fetched your branch locally and was able to build the docs successfully! 🎉

Your addition renders like this:
new_tutorial

In fact, with Sphinx-Gallery, the syntax to use in your Python files is reStructuredText (not Markdown). For example, for external links:

# `Gamma correction <https://en.wikipedia.org/wiki/Gamma_correction>`_

Do you mind updating your contribution accordingly? Then I'm happy to review it thoroughly!

@MarcSkovMadsen
Copy link
Author

Thanks @mkcor. Will do 👍

@MarcSkovMadsen
Copy link
Author

I cannot include an .mp4 in the document. But I can include a .gif. For now I upload here to get a url to include.

scikit-image-w-holoviz

@MarcSkovMadsen
Copy link
Author

Thanks @mkcor

Thanks for your patience. I got distracted. It's updated now. Have a nice Christmas.

Copy link
Member

@mkcor mkcor left a comment

Choose a reason for hiding this comment

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

Thank you, @MarcSkovMadsen!

Please improve the writeup along the lines of this quick review. The text can be very concise but it should flow well. Feel free to draw inspiration from well-written gallery examples, such as "Estimate anisotropy in a 3D microscopy image."

doc/examples/applications/plot_tool_using_holoviz.py Outdated Show resolved Hide resolved
doc/examples/applications/plot_tool_using_holoviz.py Outdated Show resolved Hide resolved
doc/examples/applications/plot_tool_using_holoviz.py Outdated Show resolved Hide resolved
doc/examples/applications/plot_tool_using_holoviz.py Outdated Show resolved Hide resolved
doc/examples/applications/plot_tool_using_holoviz.py Outdated Show resolved Hide resolved
doc/examples/applications/plot_tool_using_holoviz.py Outdated Show resolved Hide resolved
MarcSkovMadsen and others added 8 commits December 20, 2021 18:17
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
@MarcSkovMadsen
Copy link
Author

Thanks @mkcor . I've tried to apply the inspiration from plot_3d_structure_tensor.py. I hope you can provide me some more guidance. Thanks.

@mkcor
Copy link
Member

mkcor commented Jan 2, 2022

PS: @MarcSkovMadsen did you know you could apply suggested changes as a batch? https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request

@MarcSkovMadsen
Copy link
Author

Thx. I did not know.

requirements/docs.txt Outdated Show resolved Hide resolved
Copy link
Member

@mkcor mkcor left a comment

Choose a reason for hiding this comment

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

Please proofread your text and please wrap your lines at 79 characters. Let me know when you're done and I'll review! Thanks.

doc/examples/applications/plot_tool_using_holoviz.py Outdated Show resolved Hide resolved
MarcSkovMadsen and others added 3 commits February 19, 2022 16:07
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
@MarcSkovMadsen
Copy link
Author

MarcSkovMadsen commented Feb 19, 2022

I've relaxed the versions, fixed the spelling error, tried to identify more spelling errors, updated to respect 79 character limit.

In the two places below, I've not been able to respect the 79 character limit. I've tried to google to find out how I can break lines in figure and code block without luck. So I've kept them as they are.

image

Please let me know if there is more I should do @mkcor . Thanks.

Comment on lines +6 to +7
In this tutorial, we build interactive tools and an application using
`Panel <https://panel.holoviz.org>`_ and `HoloViews <https://holoviews.org>`_.
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
In this tutorial, we build interactive tools and an application using
`Panel <https://panel.holoviz.org>`_ and `HoloViews <https://holoviews.org>`_.
In this tutorial, we build interactive tools using
`Panel <https://panel.holoviz.org>`_ and `HoloViews <https://holoviews.org>`_,
two visualization libraries maintained by `HoloViz <https://holoviz.org/>`_.

Since 'HoloViz' appears in the title, it must appear in the body text at some point, preferably towards the beginning, so the reader knows what we are talking about...

Comment on lines +8 to +11

We will build an application based on the Sobel algorithm to interactively
find a great color map for the visualization. We will add some nice features,
like a download button, to make sharing your findings easier.
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
We will build an application based on the Sobel algorithm to interactively
find a great color map for the visualization. We will add some nice features,
like a download button, to make sharing your findings easier.
We build a web application based on the Sobel algorithm which lets the user
find interactively a great color map for the visualization.
Then we add interaction features such as a download button.
  • Keep the same tense.
  • What is "finding a great color map [for the visualization]" supposed to mean? Even if we're not dealing with a scientific problem, the question should be somewhat interesting, so the reader can see the point of building this app... From your animation, it looks like the app is only about changing the color map interactively (not really finding... some kind of optimum, as the verb 'finding' implies). If the app is 'based on the Sobel algorithm' as you write, then what seems 'interesting' would be for the user to vary the mode and/or cval values...
  • I think everyone knows what a download button is for... To make the comment relevant, it should specify what exactly is being downloaded upon clicking the download button in question: a CSV of the full dataset? a CSV of the subset currently displayed? a PNG of the static picture? I think it would be more fitting to specify this when we are at that point in the tutorial.

Screencast showcasing the example application.


You can develop using notebooks or .py files. You can use it with a very
Copy link
Member

Choose a reason for hiding this comment

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

What does 'it' refer to?

Comment on lines +12 to +13

These techniques can make your analyses more interactive and, hence, powerful.
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
These techniques can make your analyses more interactive and, hence, powerful.
Interactive applications provide flexible ways to visualize and explore data,
making data analysis more powerful and, at the same time, accessible to
non-technical audiences.

You might as well stick with a very generic sentence.

@mkcor
Copy link
Member

mkcor commented Feb 23, 2022

In the two places below, I've not been able to respect the 79 character limit. I've tried to google to find out how I can break lines in figure and code block without luck. So I've kept them as they are.

Sure, that's fine.

width=width,
cmap="binary_r",
title="Before",
active_tools=["box_zoom"],
Copy link
Member

Choose a reason for hiding this comment

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

@MarcSkovMadsen could we do without option 'active_tools' so that we could use a backend other than 'bokeh' and, hence/perhaps, have the figure display in the static HTML page?

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.

None yet

3 participants