Add example code for generating structuring elements. by b-z · Pull Request #4528 · scikit-image/scikit-image

b-z · 2020-03-26T09:52:05Z

Description

Closes #4431 .
The example shows how to use functions in skimage.morphology to generate structuring elements.
The title of each plot indicates the call of the function.

Checklist

~~Docstrings for all functions~~
~~Gallery example in ./doc/examples (new features only)~~
Benchmark in ./benchmarks, if your changes aren't covered by an
existing benchmark
~~Unit tests~~
Clean style in the spirit of PEP8

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.
Consider backporting the PR with @meeseeksdev backport to v0.14.x

sciunto

Great PR, I love the numbers superimposed on top of the color plot!

I'm wondering if two separate figures would be better, t get ride of the idx and also for the thumbnail. Others can provide their opinion.

sciunto · 2020-03-26T15:37:33Z

doc/examples/numpy_operations/plot_structuring_elements.py

+=============================
+
+This example shows how to use functions in :py:module:`skimage.morphology`
+to generate structuring elements.


I think this paragraph could be improved by explaining what is a structuring element and providing few examples of functions using them (like erosion, dilation, functions in rank filters)...

Hello sciunto. Thank you for pointing it out : )
I felt a bit hard to explain what structuring element is (my English is bad..).
I made this as an example, which modified from the current example of dilation function:

>>> import numpy as np >>> from skimage.morphology import dilation, square, disk >>> bright_pixel = np.array([[0, 0, 0, 0, 0], ... [0, 0, 0, 0, 0], ... [0, 0, 1, 0, 0], ... [0, 0, 0, 0, 0], ... [0, 0, 0, 0, 0]], dtype=np.uint8) >>> dilation(bright_pixel, square(3)) array([[0, 0, 0, 0, 0], [0, 1, 1, 1, 0], [0, 1, 1, 1, 0], [0, 1, 1, 1, 0], [0, 0, 0, 0, 0]], dtype=uint8) >>> dilation(bright_pixel, disk(2)) array([[0, 0, 1, 0, 0], [0, 1, 1, 1, 0], [1, 1, 1, 1, 1], [0, 1, 1, 1, 0], [0, 0, 1, 0, 0]], dtype=uint8)

Not sure if it should be put into the code.

sciunto · 2020-03-26T15:38:30Z

doc/examples/numpy_operations/plot_structuring_elements.py

+    ax = fig.add_subplot(3, 3, idx)
+    ax.imshow(struc, cmap="Paired", vmin=0, vmax=12)
+    for i in range(struc.shape[0]):
+        for j in range(struc.shape[1]):


Here we could use itertools.product to avoid nested loops, but it's optional.

soupault · 2020-03-26T18:09:15Z

Thanks for the PR! This looks great!

I think for 2D we should try keeping the same absolute scale of the pixels across the plots. For 3D, one of these visualization methods may suit better - https://matplotlib.org/3.2.0/gallery/mplot3d/voxels.html, https://matplotlib.org/3.2.0/gallery/mplot3d/3d_bars.html .

b-z · 2020-04-01T01:47:47Z

Thanks for the comments. I will improve these in my free time.

sciunto · 2020-04-17T12:05:09Z

@b-z Do you have a chance to work on this again?

@scikit-image/core As we will improve our API in the future (#4536), I propose to merge this PR soon at it is already good. We will have the occasion to revisit the example later on.

rfezzani

Thank you @b-z for this, I just have some stylistic suggestions ;-)

doc/examples/numpy_operations/plot_structuring_elements.py

Co-Authored-By: Riadh Fezzani <rfezzani@gmail.com>

Co-Authored-By: rfezzani <rfezzani@gmail.com>

rfezzani

Thank you @b-z, just two small suggestions more ;-)

doc/examples/numpy_operations/plot_structuring_elements.py

b-z · 2020-04-19T15:57:16Z

Thanks for the PR! This looks great!

I think for 2D we should try keeping the same absolute scale of the pixels across the plots. For 3D, one of these visualization methods may suit better - https://matplotlib.org/3.2.0/gallery/mplot3d/voxels.html, https://matplotlib.org/3.2.0/gallery/mplot3d/3d_bars.html .

Hello @soupault , thanks for your advice!
I tried to change them to same scale. As for the 3D visualization method proposed, I prefer the voxel plot, and also used same scale.
Do you like the following result better?

doc/examples/numpy_operations/plot_structuring_elements.py

Co-Authored-By: Riadh Fezzani <rfezzani@gmail.com>

jni · 2020-04-20T07:40:37Z

@b-z I love your latest plots! Could you push the code for those to your branch? It is still showing the dots and the different element sizes.

I also like @rfezzani's suggestions for iterating over the values. However, since you are not going to access the dictionary, it might be better to revert back to a list of tuples! Sorry about the back and forth there...!

Anyway, all that is optional, but the shaded voxels look super nice and we should get that code in since you worked it out!

rfezzani · 2020-04-20T08:02:08Z

However, since you are not going to access the dictionary,

I did not get the point. Sorry @jni, can you elaborate more on this? 🙂

sciunto · 2020-04-20T08:31:24Z

Me too. @rfezzani suggestion looks appropriate to me.

jni · 2020-04-20T08:41:30Z

@rfezzani

Currently we are doing:

structs2d = {'square(5)': square(5), ...}
...
for title, struc in structs2d.items():
    ...

I suggest:

structs2d = [('square(5)', square(5)), ...]
...
for title, struc in structs2d:
    ...

rfezzani · 2020-04-20T13:07:32Z

I understood that @jni, I just still don't get your objection:

However, since you are not going to access the dictionary,

If it is simply a matter of code style, no problem... Everybody has its own preferences ;-)

@rfezzani

Modify visulization parameters; change 3D plot method; apply @rfezzani 's suggestions. Co-Authored-By: rfezzani <rfezzani@gmail.com>

…/b-z/scikit-image into add_example_structuring_elements

b-z · 2020-04-20T13:41:06Z

@rfezzani

Currently we are doing:

structs2d = {'square(5)': square(5), ...}
...
for title, struc in structs2d.items():
    ...

I suggest:

structs2d = [('square(5)', square(5)), ...]
...
for title, struc in structs2d:
    ...

The newest code is using the style rfezzani suggested.
I'm new to open source communities, and cannot decide which to use.
Thanks 😃

jni · 2020-04-20T13:59:37Z

@rfezzani what I meant is that we are not using the mapping feature of the dictionary. We stuff things into a dictionary then immediately unpack it into tuples. So we might as well just use the tuples.

@b-z Don’t worry, this is indeed just a suggestion. It’s common for maintainers to have disagreements, and it’s something you get used to over time — you eventually develop a sense for what are requirements and what are minor suggestions. Either way is minor with this dict/tuple question. Thank you so much for your work!

jni

This is ready to go as far as I’m concerned. Remaining issues are minor and can be addressed in future PRs. Thank you for bearing with us @b-z and thanks everyone for their reviews so far!

soupault · 2020-04-20T18:26:22Z

@b-z thanks a lot for the updates! This looks amazing - #4528 (comment)! So happy to have this work merged!

Add example code for generating structuring elements.

ce97910

sciunto added the 📄 type: Documentation Updates, fixes and additions to documentation label Mar 26, 2020

sciunto requested a review from emmanuelle March 26, 2020 10:14

sciunto reviewed Mar 26, 2020

View reviewed changes

soupault added this to the 0.17 milestone Mar 26, 2020

sciunto mentioned this pull request Mar 30, 2020

Size of structuring elements a bit inconsistent #4536

Open

sciunto modified the milestones: 0.17, 0.18 Apr 8, 2020

emmanuelle mentioned this pull request Apr 12, 2020

2020's calendar of community management #4486

Closed

sciunto approved these changes Apr 17, 2020

View reviewed changes

rfezzani reviewed Apr 17, 2020

View reviewed changes

doc/examples/numpy_operations/plot_structuring_elements.py Outdated Show resolved Hide resolved

doc/examples/numpy_operations/plot_structuring_elements.py Outdated Show resolved Hide resolved

doc/examples/numpy_operations/plot_structuring_elements.py Outdated Show resolved Hide resolved

b-z and others added 4 commits April 19, 2020 22:48

Update doc/examples/numpy_operations/plot_structuring_elements.py

a77ff90

Co-Authored-By: Riadh Fezzani <rfezzani@gmail.com>

Update doc/examples/numpy_operations/plot_structuring_elements.py

4baa5d6

Co-Authored-By: Riadh Fezzani <rfezzani@gmail.com>

Update doc/examples/numpy_operations/plot_structuring_elements.py

c4acfbb

Co-Authored-By: Riadh Fezzani <rfezzani@gmail.com>

Apply @rfezzani 's stylistic suggestions.

085d6f7

Co-Authored-By: rfezzani <rfezzani@gmail.com>

rfezzani reviewed Apr 19, 2020

View reviewed changes

doc/examples/numpy_operations/plot_structuring_elements.py Outdated Show resolved Hide resolved

doc/examples/numpy_operations/plot_structuring_elements.py Outdated Show resolved Hide resolved

rfezzani reviewed Apr 19, 2020

View reviewed changes

doc/examples/numpy_operations/plot_structuring_elements.py Outdated Show resolved Hide resolved

Update doc/examples/numpy_operations/plot_structuring_elements.py

db2912c

Co-Authored-By: Riadh Fezzani <rfezzani@gmail.com>

b-z and others added 2 commits April 20, 2020 22:35

Change 3D plot method.

8a8a0ef

Modify visulization parameters; change 3D plot method; apply @rfezzani 's suggestions. Co-Authored-By: rfezzani <rfezzani@gmail.com>

Merge branch 'add_example_structuring_elements' of https://github.com…

b533c75

…/b-z/scikit-image into add_example_structuring_elements

sciunto self-requested a review April 20, 2020 13:57

sciunto approved these changes Apr 20, 2020

View reviewed changes

jni approved these changes Apr 20, 2020

View reviewed changes

jni merged commit d28bc61 into scikit-image:master Apr 20, 2020

b-z deleted the add_example_structuring_elements branch April 21, 2020 01:38

Uh oh!

Conversation

b-z commented Mar 26, 2020

Description

Checklist

For reviewers

Uh oh!

sciunto left a comment

Choose a reason for hiding this comment

Uh oh!

sciunto Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

b-z Apr 20, 2020

Choose a reason for hiding this comment

Uh oh!

sciunto Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

soupault commented Mar 26, 2020

Uh oh!

b-z commented Apr 1, 2020

Uh oh!

sciunto commented Apr 17, 2020

Uh oh!

rfezzani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rfezzani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

b-z commented Apr 19, 2020

Uh oh!

Uh oh!

jni commented Apr 20, 2020

Uh oh!

rfezzani commented Apr 20, 2020

Uh oh!

sciunto commented Apr 20, 2020

Uh oh!

jni commented Apr 20, 2020

Uh oh!

rfezzani commented Apr 20, 2020

Uh oh!

b-z commented Apr 20, 2020

Uh oh!

jni commented Apr 20, 2020

Uh oh!

jni left a comment

Choose a reason for hiding this comment

Uh oh!

soupault commented Apr 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants