-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
WSI reader #1548
WSI reader #1548
Conversation
580ef07
to
f7565bf
Compare
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
…ology_dataset Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
@wyli, @Nic-Ma
|
|
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, please see some initial comments inline, I'll try it out with an EA container soon...
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your quick update.
I put several minor comments inline, others look good to me.
And please add this new object to the "init.py" of mode/data/
and docs/sources/data.
Thanks.
for name in filenames: | ||
img = self.wsi_reader(name) | ||
if self.wsi_reader_name == "openslide": | ||
img.shape = (img.dimensions[1], img.dimensions[0], 3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it support images with channel = 1 or no channel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only supports RGB data as the output and it explicitly convert it to RGB, so the channels always should be 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, the doc suggests RGBA (https://openslide.org/api/python/#openslide.OpenSlide.associated_images), I think we should make it clear in the docstring if we only support RGB
self, | ||
img_obj, | ||
location: Tuple[int, int] = (0, 0), | ||
size: Optional[Tuple[int, int]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to use spatial_size
, otherwise, it may be confusing whether it contains channel
dim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of agree here but since both OpenSlide and cuClaraImage use size
to define this input argument, I found using another name confusing for people who has ML in pathology background and has worked with WSI in the past.
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
Signed-off-by: Behrooz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Thanks for your quick update!
Fixes #1504
Description
Training models for pathology use cases requires loading patches from Whole Slide Imaging (WSI) scans. These images are enormous in size and usually much larger than user's available RAM (#1504). This PR provide a solution by implementing whole slide image readers for CuImage and OpenSlide.
Types of changes
./runtests.sh --codeformat --coverage
../runtests.sh --quick
.make html
command in thedocs/
folder.