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

Fix unused arg in SlidingPatchWSIDataset #6047

Merged
merged 4 commits into from
Feb 22, 2023
Merged

Conversation

drbeh
Copy link
Member

@drbeh drbeh commented Feb 22, 2023

Description

This PR removes map_level from SlidingPatchWSIDataset, which seems to be left from the previous changes and has not used anywhere. Also it updates the outdated docstring.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • In-line docstrings updated.

Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
@drbeh drbeh requested review from wyli and Nic-Ma February 22, 2023 16:02
Signed-off-by: Behrooz <[email protected]>
@drbeh drbeh requested a review from wyli February 22, 2023 16:13
@wyli
Copy link
Contributor

wyli commented Feb 22, 2023

/build

@tangy5
Copy link
Contributor

tangy5 commented Feb 22, 2023

The dev on this WSI dataset class is great!
One general comment, can we have a SlidingWindowWSIDataset with Otsu masked regions in the future?
If we do direct SlidingWindow on WSI, there will be large regions (patches) are blank..
Maybe we can combine some basic functions of MaskedWSIDataset and this sliding window mechanism in the future.
Thanks

@wyli wyli merged commit 3eef61e into Project-MONAI:dev Feb 22, 2023
@drbeh
Copy link
Member Author

drbeh commented Feb 22, 2023

The dev on this WSI dataset class is great!

One general comment, can we have a SlidingWindowWSIDataset with Otsu masked regions in the future?

If we do direct SlidingWindow on WSI, there will be large regions (patches) are blank..

Maybe we can combine some basic functions of MaskedWSIDataset and this sliding window mechanism in the future.

Thanks

Hi @tangy5, absolutely! We're going to address that soon with wsi sliding window splitter. Will keep you updated.

milesial pushed a commit to dongyang0122/MONAI that referenced this pull request Feb 23, 2023
### Description

This PR removes `map_level` from `SlidingPatchWSIDataset`, which seems
to be left from the previous changes and has not used anywhere. Also it
updates the outdated docstring.

### 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] In-line docstrings updated.

---------

Signed-off-by: Behrooz <[email protected]>
@siemdejong
Copy link

Off-topic, but what is the status on a MaskedSlidingPatchWSIDataset? Or is there an easy way to do this with the currently implemented WSI datasets which I'm currently missing? Should I open a new issue?

The dev on this WSI dataset class is great!
One general comment, can we have a SlidingWindowWSIDataset with Otsu masked regions in the future?
If we do direct SlidingWindow on WSI, there will be large regions (patches) are blank..
Maybe we can combine some basic functions of MaskedWSIDataset and this sliding window mechanism in the future.
Thanks

Hi @tangy5, absolutely! We're going to address that soon with wsi sliding window splitter. Will keep you updated.

@KumoLiu
Copy link
Contributor

KumoLiu commented Oct 12, 2023

Hi @siemdejong, perhaps you could use this filter_fn in the WSISlidingWindowSplitter.

filter_fn: a callable to filter patches. It should accepts exactly two parameters (patch, location), and

@siemdejong
Copy link

siemdejong commented Oct 12, 2023

Thanks @KumoLiu! I was hoping for a solution that doesn't require WSISlidingWindowSplitter, as I would like use a dataset inheriting from monai.data.Dataset.

Mostly because I would prefer having a mappable Dataset and not a yielding iterator. So if only this filter_fn was available in SlidingPatchWSIDataset, that would be perfect.

Would this be something that could be easily added?

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.

5 participants