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

LabeledImageServer coordinate and downsample behaviour #26

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alanocallaghan
Copy link
Contributor

@alanocallaghan alanocallaghan commented Nov 1, 2024

Outline

Currently, LabeledImageServer has behaviour that is definitively broken with respect to coordinates, and some behaviour that is arguably unintuitive with respect to downsampling.

Coordinates

When querying a LabelledImageServer, we create a Pillow image and draw the geometries on it. This means that for any Region2D which does not begin at the origin, we are drawing geometries in a different coordinate space. To return a correct labelled image, we must first translate the geometries to remove the offset of the region request, such that (0,0) in the pillow image is the origin of the region request, rather than always being the origin of the full-sized labelled image as it currently is.

Relevant code:

def _read_block(self, level: int, region: Region2D) -> np.ndarray:
if self._multichannel:
full_image = np.zeros((self.metadata.n_channels, region.height, region.width), dtype=self.metadata.dtype)
feature_indices = self._tree.query(region.geometry)
labels = set(self._feature_index_to_label.values())
for label in labels:
image = PIL.Image.new('1', (region.width, region.height))
drawing_context = PIL.ImageDraw.Draw(image)
for i in feature_indices:
if label == self._feature_index_to_label[i]:
draw_geometry(
image.size,
drawing_context,
self._geometries[i],
1
)
full_image[label, :, :] = np.asarray(image, dtype=self.metadata.dtype)
return full_image
else:
image = PIL.Image.new('I', (region.width, region.height))
drawing_context = PIL.ImageDraw.Draw(image)
for i in self._tree.query(region.geometry):
draw_geometry(
image.size,
drawing_context,
self._geometries[i],
self._feature_index_to_label[i]
)
return np.expand_dims(np.asarray(image, dtype=self.metadata.dtype), axis=0)

Downsampling

The current behaviour is arguably unintuitive with respect to downsampling as demonstrated in the other 4 unit tests added. There is some discussion in #25 about whether this behaviour should change. In my view, if retaining the current behaviour, we should warn users (either in code or documentation but ideally both) that they may be receiving rasterised images, and that they should be supplying coordinates based on the downsampled image, rather than the full-sized image that the labeled image server is created from.

Code modifications

Currently, this PR just addresses the offset issue with coordinates.

@alanocallaghan alanocallaghan changed the title LabeledImageServer fixes LabeledImageServer coordinate and downsample behaviour Nov 1, 2024
@petebankhead
Copy link
Member

petebankhead commented Nov 1, 2024

that they should be supplying coordinates based on the downsampled image, rather than the full-sized image that the labeled image server is created from.

I’ll try this out next week, but I think I agree this behavior sounds wrong. I’d expect that the coordinates for read_region would correspond to downsample=1… but need to check what QuPath does.

It is an awkward and fiddly issue, where I think we need to be consistent in both for now - while forming an opinion for how it should look if/when we reconsider the read_region thing entirely.

It was a nice idea originally to provide pixel access at any arbitrary resolution based on the full-resolution image coordinates, but I think it breaks down in the details - both due to rounding errors, and the need to relate regions across images that correspond but have different resolutions. So I think in the short term we’re stuck looking for a least bad approach.

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.

2 participants