Skip to content

Redesign PatchWSIDataset#4152

Merged
bhashemian merged 17 commits into
Project-MONAI:devfrom
bhashemian:new-wsi-datasets
Apr 22, 2022
Merged

Redesign PatchWSIDataset#4152
bhashemian merged 17 commits into
Project-MONAI:devfrom
bhashemian:new-wsi-datasets

Conversation

@bhashemian

@bhashemian bhashemian commented Apr 20, 2022

Copy link
Copy Markdown
Member

Fixes #3930

Description

This PR refactor PatchWSIDataset and uses new WSIReader as the backend.

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 --disttests.
  • 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>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian requested review from Nic-Ma and wyli April 20, 2022 18:37
Comment thread monai/data/wsi_datasets.py Outdated
Comment thread monai/data/wsi_datasets.py Outdated
Comment thread monai/data/wsi_datasets.py Outdated
Comment thread monai/data/wsi_datasets.py
Comment thread monai/data/wsi_datasets.py Outdated
@Nic-Ma

Nic-Ma commented Apr 21, 2022

Copy link
Copy Markdown
Contributor

Hi @drbeh ,

BTW, about the SmartCachePatchWSIDataset, I think your previous suggestion was correct, we can slightly optimize the CacheDataset and SmartCacheDataset, etc. Then we can use them as decorators on another dataset object in future.

For now, you can use PatchWSIDataset as the data arg of SmartCacheDataset in your program to replace:
https://github.com/Project-MONAI/MONAI/blob/dev/monai/apps/pathology/data/datasets.py#L166

We must implement SmartCachePatchWSIDataset before because Clara 4.0 only supports 1 dataset in the config, now it's not necessary anymore.

Thanks.

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian

Copy link
Copy Markdown
Member Author

Hi @drbeh ,

BTW, about the SmartCachePatchWSIDataset, I think your previous suggestion was correct, we can slightly optimize the CacheDataset and SmartCacheDataset, etc. Then we can use them as decorators on another dataset object in future.

For now, you can use PatchWSIDataset as the data arg of SmartCacheDataset in your program to replace: https://github.com/Project-MONAI/MONAI/blob/dev/monai/apps/pathology/data/datasets.py#L166

We must implement SmartCachePatchWSIDataset before because Clara 4.0 only supports 1 dataset in the config.

Thanks.

Sure, will add SmartCache in a seperate PR as the test cases need some attention.

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>
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>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian

Copy link
Copy Markdown
Member Author

Hi @Nic-Ma, any other comment here? thanks :)

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

@Nic-Ma Nic-Ma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick update.
Looks good to me.

Comment thread monai/data/wsi_datasets.py
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@Nic-Ma

Nic-Ma commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

/black

@Nic-Ma

Nic-Ma commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

/build

@bhashemian bhashemian enabled auto-merge (squash) April 22, 2022 13:56
@bhashemian bhashemian merged commit 65d7296 into Project-MONAI:dev Apr 22, 2022
@bhashemian bhashemian deleted the new-wsi-datasets branch May 2, 2022 15:39
Can-Zhao pushed a commit to Can-Zhao/MONAI that referenced this pull request May 10, 2022
* Implement PatchWSIDataset

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add unittests

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add docs

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Reorder imports

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* formatting:

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Address comments

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update to be compatible with Dataset

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update reader to accept str, class, object

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add test cases for various reader and level arguments

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update comment about OpenSlide cache

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Rename reader_name to backend

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add new test cases

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add unittests for openslide

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add new test cases

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* sorts

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add docstring for kwargs

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor PatchWSIDataset

2 participants