8000 Port 3D image processing tutorial into gallery. by mkcor · Pull Request #4850 · scikit-image/scikit-image · GitHub
[go: up one dir, main page]

Skip to content

Port 3D image processing tutorial into gallery. #4850

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

Merged
merged 23 commits into from
Aug 3, 2020

Conversation

mkcor
Copy link
Member
@mkcor mkcor commented Jul 16, 2020

Description

In this PR, we are porting an existing tutorial, i.e., https://github.com/scikit-image/skimage-tutorials/blob/master/lectures/three_dimensional_image_processing.ipynb, into the gallery of examples. This addition falls under #4601 and makes use of the 3D dataset available with Pooch (namely, cells.tif).

I have only included the first two sections of the original tutorial:

  • Load and display 3D images
  • Adjust exposure (in experimental images)

I think that the segmentation of cells and nuclei could be its own tutorial, and so could feature extraction. Each example should be self-contained, but could nevertheless refer to another example (e.g., this introduction to 3D image processing where we only explore slices and adjust exposure).

If core devs prefer keeping everything in one place, I'm happy to keep going and append sections to the same file.

Checklist

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 Jul 16, 2020

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

Line 220:21: E703 statement ends with a semicolon
Line 281:31: E703 statement ends with a semicolon

Comment last updated at 2020-07-28 16:31:44 UTC

@mkcor
Copy link
Member Author
mkcor commented Jul 17, 2020

@emmanuelle @alexdesiqueira @jni Following up on our sprint conversations, please have a look when you get a chance. Thanks!


This tutorial is an introduction to three-dimensional image processing. Images
are represented as `numpy` arrays. A single-channel, or grayscale, image is a
2D matrix of pixel intensities of shape `(row, column)`. We can construct a 3D
Copy link
Member

Choose a reason for hiding this comment

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

since you are porting markdown to rst, there are several places like here where you need the text to be enclosed in double backticks instead of single ones.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I didn't notice because it rendered nicely (sort of):
backticks
now I realize this is italic, not code-style!

Copy link
Member

Choose a reason for hiding this comment

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

indeed :-).

Copy link
Member

Choose a reason for hiding this comment

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

but are you ok changing the single backticks to double ones?

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'm definitely ok, in the sense that I don't have the strongest opinion. But I have hesitations in terms of consistency and pedagogy: Words in code style should refer to words... in code. For example, "the shape (plane, row, column)" (lines 9--10) should match the code we have line 98:

- (n_plane, n_row, n_col) = data.shape
+ (plane, row, column) = data.shape

There is some confusion because we are using "(plane, row, column)" to mean:

  • the number of planes, rows, columns (shape);
  • the dimensions (axes) as in line 34;
  • the indices (coordinates) along the plane, row, column dimensions as in the summary table, line 16.
    In this context, I think that using italic style for one of the meanings not found in code would make sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I went for the following: e8c23f9. I hope it's not too heavy in terms of wording...
In the table, coordinates use the square-bracket notation, as they should in general and recalling/announcing code line 103--105.
Alternatively, I considered keeping the italic style to describe dimensions:
dimensions
which could also make sense.


_, (a, b, c) = plt.subplots(ncols=3, figsize=(15, 5))

show_plane(a, data[32], title="Plane = 32")
Copy link
Member

Choose a reason for hiding this comment

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

maybe write

(l_plane, l_row, l_col) = data.shape

and use l_plane // 2 etc. below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, hard-coding isn't justified here. Done 9bb3123. I have used notation (n_plane, n_row, n_col) though, it comes more natural to me.

@emmanuelle
Copy link
Member

Thanks @mkcor ! I left a couple of comments. I think it's very useful to port the tutorials to the main doc, one reason being that they'll go through the traditional review process! For now I'm fine with having several tutorials instead of one very big one, as long as there is proper cross-referencing between them. See for example https://github.com/scikit-image/scikit-image/blob/master/doc/examples/applications/plot_human_mitosis.py#L50 for the syntax of such links.

At some point maybe I'll contribute a plotly paragraph to show how to visualize these data in plotly (without the need for a Jupyter kernel :-)) but I don't have the time at the moment.

@alexdesiqueira alexdesiqueira added the 📄 type: Documentation Updates, fixes and additions to documentation label Jul 25, 2020
@mkcor
Copy link
Member Author
mkcor commented Jul 27, 2020

I think it's very useful to port the tutorials to the main doc, one reason being that they'll go through the traditional review process!

Absolutely!

For now I'm fine with having several tutorials instead of one very big one, as long as there is proper cross-referencing between them.

Ok.

See for example https://github.com/scikit-image/scikit-image/blob/master/doc/examples/applications/plot_human_mitosis.py#L50 for the syntax of such links.

Looks like it's not the link you meant to share 🤔

At some point maybe I'll contribute a plotly paragraph to show how to visualize these data in plotly (without the need for a Jupyter kernel :-)) but I don't have the time at the moment.

Oh, good idea!!



(n_plane, n_row, n_col) = data.shape
_, (a, b, c) = plt.subplots(ncols=3, figsize=(15, 5))
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'm confused because the current guidelines say "Examples in the gallery should have a maximum figure width of 8 inches" but we keep using widths of 15, 16 inches throughout examples (not only this one)... 🤔 @emmanuelle

Copy link
Member

Choose a reason for hiding this comment

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

@stefanv is probably best placed to answer this question. I think it's not a huge deal because the gallery rescales the images. But it might affect font sizes, for example. Anyway, the answer is I don't know. =P

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea of where this 8 inches come from :-).

Copy link
Member

Choose a reason for hiding this comment

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

The rule of thumb should be to always take a look at the generated html...

Copy link
Member Author

Choose a reason for hiding this comment

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

The rule of thumb should be to always take a look at the generated html...

Sure. So maybe this line

Note that gallery examples should have a maximum figure width of 8 inches.

could be removed or edited.

Copy link
Member
@jni jni left a comment

Choose a reason for hiding this comment

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

Great! I've made only some very minor comments. Thank you @mkcor!



(n_plane, n_row, n_col) = data.shape
_, (a, b, c) = plt.subplots(ncols=3, figsize=(15, 5))
Copy link
Member

Choose a reason for hiding this comment

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

@stefanv is probably best placed to answer this question. I think it's not a huge deal because the gallery rescales the images. But it might affect font sizes, for example. Anyway, the answer is I don't know. =P

# highest extremes. Clipping the darkest and brightest 0.5% of pixels will
# increase the overall contrast of the image.

vmin, vmax = np.percentile(data, q=(0.5, 99.5))
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I prefer using quantiles, though it's a personal preference, consider this comment optional. =)

Suggested change
vmin, vmax = np.percentile(data, q=(0.5, 99.5))
vmin, vmax = np.quantile(data, q=(0.005, 0.995))

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 went for a popularity contest to decide and percentile won. ;-)
I searched for "cell biology" "percentile" vs "cell biology" "quantile".

Copy link
Member

Choose a reason for hiding this comment

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

I also did not know that numpy had a quantile function :-). (but I'm happy that I learned this, since I also prefer a bit quantile to percentile, but I'm fine with letting the pop contest decide.)

mkcor and others added 7 commits July 28, 2020 15:44
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
@emmanuelle
Copy link
Member

Merging, thanks a lot @mkcor!

@emmanuelle emmanuelle merged commit 798b495 into scikit-image:master Aug 3, 2020
@mkcor mkcor deleted the port-3D-tutorial branch August 31, 2020 07:14
lagru pushed a commit that referenced this pull request Jan 25, 2023
This PR is an addition to the 3D image processing introduction tutorial, which landed with #4850.
It relates to #4850 (comment) and #4762.
I've made a slice explorer with Plotly so it re
794C
nders even in the static HTML page.
In future iterations, I'll add a 3D visualization showing the position of the selected slice within the dataset.

I've chosen the density_heatmap function so I could readily use its animation_frame argument for the slider interaction (selection). This function is made for aggregation, which I'm bypassing by setting the number of bins to the data size...

Ideally, I wanted to use imshow to view the image slices (planes) but this wouldn't offer such a convenient abstraction in terms of slider interaction.

One issue when trying with this is that occasionally the Plotly plot would behave strangely when dragging the animation slider (the slider itself sometimes moved to a lower position on the screen mid-drag and the image would quit updating). Hitting the "home" button at top to reset the axes should restore it to a working state again. It might be somehow related to dragging too quickly as I never saw the issue if I just clicked around on different frames, only when dragging the slider.

Co-authored-by: Emmanuelle Gouillart <emma@plot.ly>
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
Co-authored-by: Gregory R. Lee <grlee77@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 type: Documentation Updates, fixes and additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0