-
Notifications
You must be signed in to change notification settings - Fork 681
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
Add zarr merger notebook section #1433
Conversation
Signed-off-by: Behrooz <[email protected]>
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Fixes #6006 ### Description This PR implements `ZarrAvgMerger` which can be used for patch inference. Also a use case is demonstrated [here](Project-MONAI/tutorials#1433). ### 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). - [x] New tests added to cover the changes. - [x] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. - [x] Documentation updated, tested `make html` command in the `docs/` folder. --------- Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
d63ad45
to
598d46b
Compare
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
View / edit / reply to this conversation on ReviewNB KumoLiu commented on 2023-06-30T02:19:35Z Line #4. import zarr Seems zarr didn't install successfully. https://github.com/Project-MONAI/tutorials/actions/runs/5413785783/jobs/9839865836?pr=1433#step:5:663 |
View / edit / reply to this conversation on ReviewNB KumoLiu commented on 2023-06-30T02:19:36Z I just noticed that this one maybe "effect of adding patch filtering" to be consistent with the above cells. What do you think? |
View / edit / reply to this conversation on ReviewNB KumoLiu commented on 2023-06-30T02:19:37Z I'm a little confused here, maybe add some explanation or put some official zarr links to help users understand. What do you think? |
Thanks for the update, overall looks good to me. Just leave a few comments inline. |
Signed-off-by: Behrooz <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Hi @KumoLiu, please take another look. Thanks |
This PR add an example for `ZarrAvgMerger` to be plugged into `PatchInferer` along with `WSISlidingWindowSplitter`. Note: Project-MONAI/MONAI#6633 in MONAI core should be merged before this one. ### Checks <!--- Put an `x` in all the boxes that apply, and remove the not applicable items --> - [x] Avoid including large-size files in the PR. - [x] Clean up long text outputs from code cells in the notebook. - [x] For security purposes, please check the contents and remove any sensitive info such as user names and private key. - [x] Ensure (1) hyperlinks and markdown anchors are working (2) use relative paths for tutorial repo files (3) put figure and graphs in the `./figure` folder - [x] Notebook runs automatically `./runner.sh -t <path to .ipynb file>` --------- Signed-off-by: Behrooz <[email protected]>
This PR add an example for
ZarrAvgMerger
to be plugged intoPatchInferer
along withWSISlidingWindowSplitter
.Note: Project-MONAI/MONAI#6633 in MONAI core should be merged before this one.
Checks
./figure
folder./runner.sh -t <path to .ipynb file>