8000 Add expand_labels function to dilate segments while avoiding overlaps by VolkerH · Pull Request #4795 · scikit-image/scikit-image · GitHub
[go: up one dir, main page]
Skip to content

Add expand_labels function to dilate segments while avoiding overlaps #4795

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 39 commits into from
Jun 29, 2020
Merged

Add expand_labels function to dilate segments while avoiding overlaps #4795

merged 39 commits into from
Jun 29, 2020

Conversation

VolkerH
Copy link
Contributor
@VolkerH VolkerH commented Jun 19, 2020

Description

This implements growing of connected component regions in a label array up to a specified number of pixels/voxels, but not starting to merge/overlap separate regions.

This functionality is similar to the Distance N method in Cellprofiler's IdentifySecondaryObjects module, in fact I adapted the code from there but modified it to support arbitrary dimensions (that should tick a box with @jni). Also see the discussion on image.sc.
I am not aware of an original paper I can cite for this, but there probably is one.

I created a demo notebook that shows what this is doing. You can see the demo notebook here.

This is still a WIP.
Tests, examples, and formatting still to do.

Checklist

  • (started) Docstrings for all functions
  • (TODO) Gallery example in ./doc/examples (new features only)
  • (TODO) Benchmark in ./benchmarks, if your changes aren't covered by an
    existing benchmark
  • (TODO) Unit tests
  • (TODO) 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.

@pep8speaks
Copy link
pep8speaks commented Jun 19, 2020 •
edited
Loading

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

Line 19:80: E501 line too long (85 > 79 characters)
Line 19:86: W291 trailing whitespace
Line 20:80: E501 line too long (87 > 79 characters)
Line 22:80: E501 line too long (84 > 79 characters)
Line 24:80: E501 line too long (86 > 79 characters)
Line 24:87: W291 trailing whitespace
Line 25:80: E501 line too long (83 > 79 characters)
Line 25:84: W291 trailing whitespace
Line 27:1: W293 blank line contains whitespace
Line 33:80: E501 line too long (81 > 79 characters)
Line 43:80: E501 line too long (91 > 79 characters)
Line 45:57: W291 trailing whitespace
Line 47:80: E501 line too long (85 > 79 characters)
Line 57:80: E501 line too long (110 > 79 characters)
Line 62:80: E501 line too long (155 > 79 characters)

Line 13:80: E501 line too long (83 > 79 characters)
Line 22:2: W291 trailing whitespace
Line 24:68: W291 trailing whitespace
Line 28:80: E501 line too long (82 > 79 characters)
Line 44:7: E126 continuation line over-indented for hanging indent
Line 54:8: E126 continuation line over-indented for hanging indent
Line 58:7: E126 continuation line over-indented for hanging indent
Line 79:7: E126 continuation line over-indented for hanging indent
Line 86:7: E126 continuation line over-indented for hanging indent
Line 106:22: E225 missing whitespace around operator
Line 107:7: E126 continuation line over-indented for hanging indent
Line 127:80: E501 line too long (80 > 79 characters)
Line 129:1: E302 expected 2 blank lines, found 1
Line 130:53: W291 trailing whitespace
Line 132:5: E122 continuation line missing indentation or outdented
Line 133:5: E122 continuation line missing indentation or outdented
Line 134:5: E122 continuation line missing indentation or outdented
Line 135:5: E122 continuation line missing indentation or outdented
Line 136:5: E122 continuation line missing indentation or outdented
Line 137:5: E122 continuation line missing indentation or outdented
Line 149:1: W293 blank line contains whitespace
Line 175:73: W291 trailing whitespace
Line 176:60: W291 trailing whitespace
Line 177:36: W291 trailing whitespace
Line 185:1: W293 blank line contains whitespace

Comment last updated at 2020-06-29 00:04:46 UTC

@VolkerH
Copy link
Contributor Author
VolkerH commented Jun 19, 2020

A question I have is:

Should this go into skimage.measure (I chose that because that's where label lives) or preferably somewhere else ?

@VolkerH VolkerH changed the title added expand_labels [WIP] added expand_labels Jun 19, 2020
@rfezzani rfezzani mentioned this pull request Jun 22, 2020
Closed
@VolkerH VolkerH marked this pull request as ready for review June 23, 2020 04:13
@VolkerH
Copy link
Contributor Author
VolkerH commented Jun 23, 2020

Tests, documentation and gallery example have been added with contributions from @jni.
This is now ready for review.

@VolkerH VolkerH changed the title [WIP] added expand_labels added expand_labels Jun 23, 2020
@jni
Copy link
Member
jni commented Jun 23, 2020

@scikit-image/core This is ready for review! Test failures are:

  • sphinx link error in the See Also section (working to fix it)
  • matplotlib 3.3.0rc1 breaking things

For reference, here is the output of the gallery example:

expand_labels

@VolkerH
Copy link
Contributor Author
VolkerH commented Jun 23, 2020 •
edited
Loading

unrelated to this pull request, but looking at the example screenshot: what is the issue with label2rgb that makes the overlay of the background greyish for the dilated labels, something to do with automated contrast scaling?

@jni
Copy link
Member
jni commented Jun 23, 2020

What is the issue with label2rgb that makes the overlay of the background greyish for the dilated labels

I expect we've done something wrong with the background label, which is supposed to be transparent...

jni
jni reviewed Jun 23, 2020
View reviewed changes
@jni
Copy link
Member
jni commented Jun 23, 2020

Fixed! This is now definitely ready for review, @scikit-image/core! =) Again, the remaining failure is unrelated to this PR and due to the matplotlib pre-release.

rfezzani
rfezzani reviewed Jun 23, 2020
View reviewed changes
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.

Thank you @VolkerH, I simply left some minor comments and the changes in _labels.py must be reverted, otherwise, it looks good ;)

VolkerH and others added 6 commits June 23, 2020 20:15
@VolkerH @rfezzani
Update skimage/segmentation/tests/test_expand_labels.py
82c2c04 
array->np.array

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@VolkerH @rfezzani
Update skimage/segmentation/tests/test_expand_labels.py
a4014f5 
another array->np.array

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@VolkerH @rfezzani
Update skimage/segmentation/tests/test_expand_labels.py
5401e1d 
pep8 blank line

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@VolkerH @rfezzani
Update skimage/segmentation/_expand_labels.py
e97d3d2 
Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@VolkerH @rfezzani
Update skimage/measure/_label.py
0e6c319 
removed unused import (numpy)

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@VolkerH @rfezzani
Update skimage/segmentation/_expand_labels.py
fa0f14f 
removed reference to image.sc discussion thread

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
jni
jni reviewed Jun 25, 2020
View reviewed changes
jni
jni reviewed Jun 25, 2020
View reviewed changes
@jni
Copy link
Member
jni commented Jun 25, 2020

I've left some suggestions where there was incorrect formatting in the docstring, but I don't think off the top of my head that these should have caused errors. Anyway if I get a chance I'll try to do a local doc build.

VolkerH and others added 6 commits June 25, 2020 12:01
@VolkerH @jni
Update skimage/segmentation/_expand_labels.py
9637cca 
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
@VolkerH @jni
Update skimage/segmentation/_expand_labels.py
7e282e7 
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
@VolkerH @jni
Update skimage/segmentation/_expand_labels.py
be1c4e9 
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
@VolkerH @jni
Update skimage/segmentation/_expand_labels.py
209b085 
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
@VolkerH @jni
Update skimage/segmentation/_expand_labels.py
28fa9ab 
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
@VolkerH @jni
Update skimage/segmentation/_expand_labels.py
6f0e123 
Co-authored-by: Juan Nunez-Iglesias <juan.nunez-iglesias@monash.edu>
@VolkerH
Copy link
Contributor Author
VolkerH commented Jun 25, 2020

Thanks @jni, I need to read up on sphinx docstring syntax.
Commited all your changes, let's see whether it makes a difference to the CI builds.

@VolkerH
Copy link
Contributor Author
VolkerH commented Jun 25, 2020

I think I found the culprit, a single whitespace introduced in the docstring of the gallery example (indentation). Hope the build goes through now.

@jni
Copy link
Member
jni commented Jun 25, 2020

It looks happy! My original approval stands. =D

emmanuelle
emmanuelle reviewed Jun 27, 2020
View reviewed changes
emmanuelle
emmanuelle reviewed Jun 27, 2020
View reviewed changes
@emmanuelle
Copy link
Member

Thank you very much @VolkerH ! I left a couple of small comments (the most important one is whether we want distance to be a kwarg with a default value of 1), and some comments of @rfezzani have not been addressed yet. But we're very close to merging I think!

VolkerH and others added 5 commits June 29, 2020 09:48
@VolkerH @emmanuelle
Update skimage/measure/_label.py
5353687 
remove blank line as suggested in review

Co-authored-by: Emmanuelle Gouillart <emma@plot.ly>
@VolkerH
added default value of 1 for distance
a759270
@VolkerH @rfezzani
Update skimage/segmentation/tests/test_expand_labels.py
f8cb820 
remove whitespace

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@VolkerH @rfezzani
Update skimage/segmentation/tests/test_expand_labels.py
3fe64cf 
chain decortators instead of using `product` as suggested in review

Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@VolkerH
removed import product
a15a602
@VolkerH
Copy link
Contributor Author
VolkerH commented Jun 29, 2020

Thanks for having a look @emmanuelle, I applied your suggestions.

Thanks also for pointing out that I overlooked some of @rfezzani 's comments (Apologies! It turned out that Github had truncated those and I had to expand them to make them visible).

All tests pass locally, so I hope the CI will also work.

Thanks everyone for their feedback.

@jni jni changed the title added expand_labels Add expand_labels function to dilate segments while avoiding overlaps Jun 29, 2020
@jni jni merged commit 477dd75 into scikit-image:master Jun 29, 2020
@jni
Copy link
Member
jni commented Jun 29, 2020

Here we go! Another excellent contribution @VolkerH! =D

@asasama12
Copy link

Naive question: when will this function be available in the main scikit-image python package?

@grlee77
Copy link
Contributor
grlee77 commented Jul 6, 2020

Naive question: when will this function be available in the main scikit-image python package?

It should appear in an upcoming scikit-image 0.18 is release later this year (I don't think we have a specific date at this point).

@jni jni mentioned this pull request Aug 26, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefanv stefanv stefanv left review comments

@emmanuelle emmanuelle emmanuelle left review comments

@jni jni jni approved these changes

@rfezzani rfezzani rfezzani approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@VolkerH @pep8speaks @jni @stefanv @emmanuelle @asasama12 @grlee77 @rfezzani
0