Skip to content

Add Classification Result Post Process for HoVerNet#5186

Merged
Nic-Ma merged 60 commits intoProject-MONAI:devfrom
KumoLiu:add-whole-postprocess
Nov 7, 2022
Merged

Add Classification Result Post Process for HoVerNet#5186
Nic-Ma merged 60 commits intoProject-MONAI:devfrom
KumoLiu:add-whole-postprocess

Conversation

@KumoLiu
Copy link
Copy Markdown
Contributor

@KumoLiu KumoLiu commented Sep 21, 2022 •
edited
Loading

Signed-off-by: KumoLiu [email protected]

Fixes #5180.

Description

Add classification post-process script for HoVerNet.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • 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 --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

KumoLiu and others added 7 commits September 21, 2022 15:22
@KumoLiu
first commit
d1d81cc 
Signed-off-by: KumoLiu <[email protected]>
@KumoLiu
add unittest
4136ec7 
Signed-off-by: KumoLiu <[email protected]>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
93437f4 
for more information, see https://pre-commit.ci
@KumoLiu
add dictionary tranform
ae0862c 
Signed-off-by: KumoLiu <[email protected]>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
1c99929 
for more information, see https://pre-commit.ci
@KumoLiu
Merge remote-tracking branch 'origin/dev' into add-whole-postprocess
bc5cf91
@KumoLiu
Merge branch 'add-whole-postprocess' of https://github.com/KumoLiu/MONAI
69df77c 
 into add-whole-postprocess
@KumoLiu KumoLiu marked this pull request as ready for review September 21, 2022 15:23
@KumoLiu KumoLiu changed the title [WIP] Add whole post-process script for HoVerNet Add whole post-process script for HoVerNet Sep 21, 2022
bhashemian
bhashemian requested changes Sep 21, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@bhashemian bhashemian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @KumoLiu, thanks for submitting this PR. it seems that this PR is still not ready for reviewing yet. Can you remove redundant files, review and address the checklist in the PR description, resolve the failed tests, and update docs before moving forward with review? Thanks

@KumoLiu
Copy link
Copy Markdown
Contributor Author

KumoLiu commented Sep 22, 2022

Hi @drbeh, I have reviewed items in this PR description, check everything that I have already done, and removed irrelevant lines. Some tests will continue to be unsuccessful since #5173 hasn't been merged. But could you please help review this PR first and let me know if you have any advice? Thanks in advance!

@KumoLiu KumoLiu requested a review from JHancox September 22, 2022 08:28
bhashemian
bhashemian reviewed Sep 23, 2022
View reviewed changes
bhashemian
bhashemian reviewed Sep 27, 2022
View reviewed changes
@KumoLiu KumoLiu requested a review from bhashemian October 28, 2022 13:21
@KumoLiu KumoLiu changed the title [WIP] Add Classification Result Post Process for HoVerNet Add Classification Result Post Process for HoVerNet Oct 28, 2022
bhashemian
bhashemian reviewed Nov 1, 2022
View reviewed changes
@bhashemian bhashemian self-requested a review November 1, 2022 20:35
bhashemian
bhashemian reviewed Nov 1, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@bhashemian bhashemian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you using the testing data? tests/testing_data/hovernet_test_data_raw.npz
If not, please remove it. Thanks

@KumoLiu
Copy link
Copy Markdown
Contributor Author

KumoLiu commented Nov 2, 2022

Are you using the testing data? tests/testing_data/hovernet_test_data_raw.npz If not, please remove it. Thanks

Yes, thanks for pointing it out, I removed it in the latest commit!

pre-commit-ci bot and others added 5 commits November 2, 2022 11:11
@pre-commit-ci @KumoLiu
[pre-commit.ci] auto fixes from pre-commit.com hooks
612a5aa 
for more information, see https://pre-commit.ci

Signed-off-by: KumoLiu <[email protected]>
@KumoLiu
fix flake8
7a4cafb 
Signed-off-by: KumoLiu <[email protected]>
@KumoLiu
remove test data
495c516 
Signed-off-by: KumoLiu <[email protected]>
@monai-bot @wyli @KumoLiu
auto updates (Project-MONAI#5445)
c615e56 
Signed-off-by: monai-bot <[email protected]>

- auto style fixing
- bump versioneer to 0.23 from 0.19

Signed-off-by: monai-bot <[email protected]>
Signed-off-by: Wenqi Li <[email protected]>
Co-authored-by: Wenqi Li <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
@KumoLiu @bhashemian
Update monai/apps/pathology/transforms/post/array.py
3669e4a 
Co-authored-by: Behrooz Hashemian <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
@KumoLiu KumoLiu force-pushed the add-whole-postprocess branch from 1cd935d to 3669e4a Compare November 2, 2022 03:12
@bhashemian
Copy link
Copy Markdown
Member

bhashemian commented Nov 2, 2022

@JHancox do you have any comments on this PR?

@KumoLiu KumoLiu mentioned this pull request Nov 3, 2022
Merged
7 tasks
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Nov 7, 2022

Hi @drbeh ,

I see @JHancox approved the PR, and @KumoLiu is developing the whole wrapper transform in #5462 .
Do you still have any other concerns? If not, I will try to merge this PR ASAP.

Thanks in advance.

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Nov 7, 2022

/black

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Nov 7, 2022

/build

1 similar comment
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Nov 7, 2022

/build

@Nic-Ma Nic-Ma enabled auto-merge (squash) November 7, 2022 14:07
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Nov 7, 2022

/build

@Nic-Ma Nic-Ma merged commit 9644b07 into Project-MONAI:dev Nov 7, 2022
@KumoLiu KumoLiu deleted the add-whole-postprocess branch November 8, 2022 02:46
bhashemian added a commit that referenced this pull request Nov 16, 2022
@KumoLiu @bhashemian
Classification post-process for HoVerNet (#5462)
b36c8db 
Signed-off-by: KumoLiu <[email protected]>

Fixes #5180.

### Description
For the post-process in HoVerNet, now we have PR #5173 to process the
segmentation output(which has been merged) and PR #5186 to process the
classification output(which is under review now). We want a whole
transform containing these two components which can be easily used in
the MONAI model zoo bundle. Here has a draft version of how to combine
these components, if anyone has any suggestions, please feel free to
take it out.

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] 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 --disttests`.
- [x] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: KumoLiu <[email protected]>
Signed-off-by: yunliu <[email protected]>
Co-authored-by: Behrooz Hashemian <[email protected]>
bhashemian added a commit to bhashemian/MONAI that referenced this pull request Nov 23, 2022
@KumoLiu @bhashemian
Classification post-process for HoVerNet (Project-MONAI#5462)
5a64a1f 
Signed-off-by: KumoLiu <[email protected]>

Fixes Project-MONAI#5180.

### Description
For the post-process in HoVerNet, now we have PR Project-MONAI#5173 to process the
segmentation output(which has been merged) and PR Project-MONAI#5186 to process the
classification output(which is under review now). We want a whole
transform containing these two components which can be easily used in
the MONAI model zoo bundle. Here has a draft version of how to combine
these components, if anyone has any suggestions, please feel free to
take it out.

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] 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 --disttests`.
- [x] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: KumoLiu <[email protected]>
Signed-off-by: yunliu <[email protected]>
Co-authored-by: Behrooz Hashemian <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rijobro rijobro rijobro left review comments

@bhashemian bhashemian bhashemian approved these changes

@JHancox JHancox JHancox approved these changes

@wyli wyli Awaiting requested review from wyli

@Nic-Ma Nic-Ma Awaiting requested review from Nic-Ma Nic-Ma is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Post-process script for HoVerNet

6 participants

@KumoLiu @bhashemian @Nic-Ma @rijobro @JHancox @monai-bot