Add RAG threshold parameters to allow return labels and masking by kheffah · Pull Request #300 · histolab/histolab

kheffah · 2021-07-23T21:20:37Z

Description

Flexible rag filter allowing:

Masked filtering to exclude, for example, areas outside the tissue
Returning labeled mask for individual clustered segments instead of a colored RGB. That way, the returned clusters can be used in downstream analysis.

Types of Changes

Issues Fixed or Closed by This PR

None

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

codecov · 2021-07-23T21:27:23Z

Codecov Report

Merging #300 (063b104) into master (29ebb9a) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #300   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1328      1331    +3     
  Branches       133       134    +1     
=========================================
+ Hits          1328      1331    +3

Impacted Files	Coverage Δ
histolab/filters/image_filters.py	`100.00% <100.00%> (ø)`
histolab/filters/image_filters_functional.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29ebb9a...063b104. Read the comment docs.

alessiamarcolini

Some comments about docstring style and testing best practices

kheffah · 2021-07-26T18:08:44Z

@alessiamarcolini OK, all done 👍🏻

alessiamarcolini · 2021-07-26T18:16:35Z

@kheffah could you please squash all the "update" commit into a single one? Thanks!!

PS there is a way to add all the suggestions to a single commit from the interface directly

kheffah · 2021-07-26T19:43:04Z

@alessiamarcolini Oops I didn't know that. OK, now commit aaf4a09 contains the squashed changes.

alessiamarcolini · 2021-07-29T10:04:40Z

Hi @kheffah, sorry I meant to do an interactive rebase to rewrite history and have only the relevant commits (with a proper message) to have a clean history. Please let me know if you need any help with this :)

kheffah · 2021-07-29T15:38:47Z

Hi @kheffah, sorry I meant to do an interactive rebase to rewrite history and have only the relevant commits (with a proper message) to have a clean history. Please let me know if you need any help with this :)

Yes please, I've never done that before. I'm dealing with a deadline at the moment so maybe at least a few days till I get to it. If you prefer to get it done quickly, feel free to do it :)

kheffah · 2021-07-31T22:28:29Z

@alessiamarcolini OK I officially give up 😆 Everytime I do the interactive rebase locally I get a conflict, asks me to resolve it, asks me to pull, and then when the rebase succeeds and I push, I still see the commits. I could probably fix this if I spend more time on it but I have high priority research tasks at the time being. But feel free to do it if you'd like, sorry about that.

ernestoarbitrio

per comments

alessiamarcolini

Last stylistic changes :)

PS after my rebase (which rewrote history) is better for you if you work on a different clone of the repo (only required for upcoming work on this branch)

ernestoarbitrio · 2021-08-02T15:32:19Z

@kheffah I think after these last comments from me and @alessiamarcolini this PR should be ready to go :D Thanks for your work

alessiamarcolini

I also asked a review to @nicolebussola if she has time ;)

ernestoarbitrio · 2021-08-02T15:38:44Z

I also asked a review to @nicolebussola if she has time ;)

Oh ... got it, but I think 2 reviews are more than enough 😄

kheffah · 2021-08-02T17:05:18Z

@alessiamarcolini @ernestoarbitrio Thanks! All suggestions incorporated now.

alessiamarcolini

LGTM

nicolebussola

Nice improvement

…ed mask for individual clustered segments

Co-authored-by: Alessia Marcolini <98marcolini@gmail.com>

alessiamarcolini requested review from alessiamarcolini, ernestoarbitrio and nicolebussola and removed request for ernestoarbitrio July 26, 2021 09:20

alessiamarcolini requested changes Jul 26, 2021

View reviewed changes

alessiamarcolini added this to the 0.2.8 milestone Jul 26, 2021

alessiamarcolini added the enhancement New feature or request label Jul 26, 2021

alessiamarcolini force-pushed the flexible-rag branch 2 times, most recently from a7e69b5 to 3607636 Compare August 2, 2021 15:14

ernestoarbitrio requested changes Aug 2, 2021

View reviewed changes

Comment thread src/histolab/filters/image_filters.py Outdated

Comment thread src/histolab/filters/image_filters_functional.py Outdated

Comment thread tests/integration/test_image_filters.py Outdated

Comment thread tests/integration/test_image_filters.py Outdated

alessiamarcolini requested changes Aug 2, 2021

View reviewed changes

Comment thread tests/integration/test_image_filters.py

Comment thread src/histolab/filters/image_filters_functional.py Outdated

Comment thread src/histolab/filters/image_filters.py Outdated

Comment thread src/histolab/filters/image_filters.py Outdated

alessiamarcolini reviewed Aug 2, 2021

View reviewed changes

ernestoarbitrio reviewed Aug 2, 2021

View reviewed changes

Comment thread src/histolab/filters/image_filters_functional.py

ernestoarbitrio approved these changes Aug 2, 2021

View reviewed changes

alessiamarcolini approved these changes Aug 10, 2021

View reviewed changes

nicolebussola approved these changes Aug 10, 2021

View reviewed changes

alessiamarcolini added the ready-for-merge label Aug 10, 2021

alessiamarcolini modified the milestones: 0.2.8, 0.4.0 Nov 8, 2021

alessiamarcolini force-pushed the flexible-rag branch from b26aaca to c08b923 Compare November 12, 2021 16:28

kheffah and others added 8 commits November 15, 2021 13:31

flexible rag filter allowing: 1. masked filtering, 2. returning label…

39bb0f2

…ed mask for individual clustered segments

fix assert_called_once_with for rag_threhshold

010b0ae

Docstring improvements

e0414cb

Co-authored-by: Alessia Marcolini <98marcolini@gmail.com>

avoid conditional in test_rag_threshold_filter_return_labels

2fd94b4

Docstring improvements

73cefdc

Co-authored-by: Alessia Marcolini <98marcolini@gmail.com>

Fix RAG threshold returns

1810545

Co-authored-by: Alessia Marcolini <98marcolini@gmail.com>

stylistic changes from code review

52b21f9

Update expectation after updating to skimage 0.18.3

063b104

alessiamarcolini force-pushed the flexible-rag branch from c08b923 to 063b104 Compare November 15, 2021 12:32

alessiamarcolini merged commit 97abd62 into master Nov 15, 2021

alessiamarcolini deleted the flexible-rag branch November 15, 2021 13:55

Uh oh!

Conversation

kheffah commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

Uh oh!

codecov Bot commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alessiamarcolini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kheffah commented Jul 26, 2021

Uh oh!

alessiamarcolini commented Jul 26, 2021

Uh oh!

kheffah commented Jul 26, 2021

Uh oh!

alessiamarcolini commented Jul 29, 2021

Uh oh!

kheffah commented Jul 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kheffah commented Jul 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ernestoarbitrio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alessiamarcolini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ernestoarbitrio commented Aug 2, 2021

Uh oh!

alessiamarcolini left a comment

Choose a reason for hiding this comment

Uh oh!

ernestoarbitrio commented Aug 2, 2021

Uh oh!

Uh oh!

kheffah commented Aug 2, 2021

Uh oh!

alessiamarcolini left a comment

Choose a reason for hiding this comment

Uh oh!

nicolebussola left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kheffah commented Jul 23, 2021 •

edited

Loading

codecov Bot commented Jul 23, 2021 •

edited

Loading

kheffah commented Jul 29, 2021 •

edited

Loading

kheffah commented Jul 31, 2021 •

edited

Loading