And endpoint to retrieve associated images and thumbnails #149
Conversation
Also send a function to the templates to convert image names into URLs to call the new assoicated image endpoint.
| <div class="inline-flex"> | ||
| <img src="{{ image_url(child_image.name, 'label') }}" alt="label image" /> | ||
| <img src="{{ image_url(child_image.name, 'macro') }}" alt="macro image" /> | ||
| <img src="{{ image_url(child_image.name, 'thumbnail') }}" | ||
| alt="thumbnail image" /> | ||
| <i class="mdi mdi-image text-sky-800"></i> | ||
| {{ child_image.name }} | ||
| </div> |
There was a problem hiding this comment.
I expect to remove this before the PR is merged, but this could be useful for reviewers testing the current GUI using the new image endpoints.
There was a problem hiding this comment.
This was handy. It shows that on TCGA-5N-A9KM-01Z-00-DX1.5197F750-D17F-459B-B74D-846F5F50F7B7.svs I'd expect to get a label image, but don't. Overall, this PR looks good.
There was a problem hiding this comment.
You can find a copy of the mentioned file on my local server under /collection/TCGA/square
There was a problem hiding this comment.
I'm a little bit confused about this image. There is indeed a label image, (Directory 5 when looking at the output of tifftools dump) but it doesn't have an ImageDescription tag pointing that out.
The value of NewSubfileType for this directory is 1, but that seems odd. According to tifftools, this bit represents "Image is a reduced-resolution version of another image in this TIFF file," but the label image isn't a reduced resolution version of any image in the file (that I'm aware of).
Additionally, other directories in the image, apart from the label and macro images, have NewSubfileType = 0. I would expect all but the highest resolution image to hava a value of 1. What's going on here? Is there another heuristic for finding a label image among the IFDs? If large image does this well, could you point me in that direction?
There was a problem hiding this comment.
Based on our conversation on Tuesday, I updated the logic in ad0302d
| def iter_ifds( | ||
| ifds: list[IFD], | ||
| tag_set=tifftools.constants.Tag, | ||
| ) -> Generator[IFD, None, None]: | ||
| for ifd in ifds: | ||
| for tag_id, entry in ifd["tags"].items(): | ||
| tag: tifftools.TiffTag = tifftools.constants.get_or_create_tag( | ||
| tag_id, | ||
| tagSet=tag_set, | ||
| datatype=tifftools.Datatype[entry["datatype"]], | ||
| ) | ||
| if tag.isIFD(): | ||
| # entry['ifds'] contains a list of lists | ||
| # see tifftools.read_tiff | ||
| for sub_ifds in entry.get("ifds", []): | ||
| yield from iter_ifds(sub_ifds, tag.get("tagset")) | ||
| yield ifd |
There was a problem hiding this comment.
It looks like an IFD iterator would be useful outside of the RedactionPlan classes. I think refactoring could be done in a follow up PR to keep the scope of this one down.
imagedephi/utils/tiff.py
Outdated
| def get_is_svs(image_path: Path) -> bool: | ||
| image_info = tifftools.read_tiff(image_path) | ||
| if tifftools.Tag.ImageDescription.value not in image_info["ifds"][0]: | ||
| return False | ||
| image_description = image_info["ifds"][0]["tags"][tifftools.Tag.ImageDescription.value]["data"] | ||
| return "aperio" in str(image_description).lower() |
There was a problem hiding this comment.
@manthey We've recently seen svs images where associated image IFDs don't have an image description. Is the first IFD always going to be a tiled image? This function could just iterate over all IFDs and return True if any ImageDescription contains "Aperio"
|
I've mirrored our logic for determining label/macro images in girder/large_image#1291. |
|
This is looking good for what is here. @naglepuff Do you want to remove the UI integration and then merge this? Or should we create a fall-back reader for a tiled layer that doesn't decode with PIL-as-a-tiff first? |
I'll take out the UI elements and merge in the rest. |
Fix #138
Caveats:
ImageDescription, which can findlabelandmacroimages for svs files, but not always. It falls back toNewSubfileType, which can findmacroimagesUI integration is for demonstration/testing, and will likely not make it through the review process for this PR, as a UI overhaul is in progress, and the new endpoint can be integrated in to that as part of a separate PR.