-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Conversation
Hello @mkcor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2020-07-28 16:31:44 UTC |
@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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed :-).
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
which could also make sense.
|
||
_, (a, b, c) = plt.subplots(ncols=3, figsize=(15, 5)) | ||
|
||
show_plane(a, data[32], title="Plane = 32") |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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. |
Co-authored-by: Emmanuelle Gouillart <emma@plot.ly>
Co-authored-by: Emmanuelle Gouillart <emma@plot.ly>
Absolutely!
Ok.
Looks like it's not the link you meant to share 🤔
Oh, good idea!! |
|
||
|
||
(n_plane, n_row, n_col) = data.shape | ||
_, (a, b, c) = plt.subplots(ncols=3, figsize=(15, 5)) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 :-).
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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
Line 418 in ebb53a6
Note that gallery examples should have a maximum figure width of 8 inches. |
could be removed or edited.
There was a problem hiding this 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)) |
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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. =)
vmin, vmax = np.percentile(data, q=(0.5, 99.5)) | |
vmin, vmax = np.quantile(data, q=(0.005, 0.995)) |
There was a problem hiding this comment.
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"
.
There was a problem hiding this comment.
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.)
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>
Merging, thanks a lot @mkcor! |
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>
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:
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
./doc/examples
(new features only)./benchmarks
, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py
.doc/release/release_dev.rst
.