Skip to content
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

Refactor Stain Normalization #3505

Draft
wants to merge 13 commits into
base: dev
Choose a base branch
from
Draft

Conversation

drbeh
Copy link
Member

@drbeh drbeh commented Dec 18, 2021

Description

This PR refactor stain normalization to support channel first input (instead of channel last), and modifies the code to be more readable with more meaningful names.

Status

Ready

Types of changes

  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh drbeh requested review from Nic-Ma and wyli December 18, 2021 01:33
@drbeh drbeh requested review from Nic-Ma and wyli and removed request for Nic-Ma and wyli February 23, 2022 21:02
@wyli
Copy link
Contributor

wyli commented Feb 24, 2022

Hi @drbeh there's a discussion about rearranging the pathology modules into monai.transforms, monai.data etc, perhaps we can include those changes together within this one? (please see https://github.com/Project-MONAI/MONAI/milestone/36)

also do we want to keep the previous names as alias so that it's backward compatible?

@drbeh
Copy link
Member Author

drbeh commented Feb 24, 2022

Hi @drbeh there's a discussion about rearranging the pathology modules into monai.transforms, monai.data etc, perhaps we can include those changes together within this one? (please see https://github.com/Project-MONAI/MONAI/milestone/36)

also do we want to keep the previous names as alias so that it's backward compatible?

Hi @wyli, that's right! but it would make it much harder for reviewing if we combine rearranging all components and refactoring this component together.
This PR is ready for review so, while I'm working on the re-organizing pathology components, why don't we review and merge it?

@drbeh
Copy link
Member Author

drbeh commented Mar 23, 2022

@wyli can we have this PR reviewed and merged?

@wyli
Copy link
Contributor

wyli commented Mar 23, 2022

Hi, we haven't finished the discussions in overall design and naming convention #3868, please could you work on that first?

@drbeh
Copy link
Member Author

drbeh commented Mar 23, 2022

Hi, we haven't finished the discussions in overall design and naming convention #3868, please could you work on that first?

Hi @wyli, I'm not sure how it is related. Can you let me know what exactly you want for this stain normalizer specifically?
I can revert the name of the components here if that's the problem.

@drbeh
Copy link
Member Author

drbeh commented Mar 24, 2022

Hi, we haven't finished the discussions in overall design and naming convention #3868, please could you work on that first?

Hi @wyli, I'm not sure how it is related. Can you let me know what exactly you want for this stain normalizer specifically? I can revert the name of the components here if that's the problem.

Actually, never mind! We'll review it when we are adding it to monai core.

@drbeh drbeh marked this pull request as draft May 29, 2022 15:56
@drbeh drbeh added Pathology/Microscopy Digital Pathology and Microscopy related and removed WG: Pathology labels May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pathology/Microscopy Digital Pathology and Microscopy related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants