From 32bb27c7e0b165a8db8485764a8137fffa03617c Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Sat, 26 Oct 2024 13:45:38 -0700 Subject: [PATCH 1/4] Improve error message when accessing a closed ImageSlide Suggested-by: Jan Harkes Signed-off-by: Benjamin Gilbert --- openslide/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openslide/__init__.py b/openslide/__init__.py index 8994bcdf..4f354b50 100644 --- a/openslide/__init__.py +++ b/openslide/__init__.py @@ -406,7 +406,7 @@ def level_dimensions(self) -> tuple[tuple[int, int]]: level_dimensions[n] contains the dimensions of level n.""" if self._image is None: - raise ValueError('Passing closed slide object') + raise ValueError('Cannot read from a closed slide') return (self._image.size,) @property @@ -444,7 +444,7 @@ def read_region( level: the level number. size: (width, height) tuple giving the region size.""" if self._image is None: - raise ValueError('Passing closed slide object') + raise ValueError('Cannot read from a closed slide') if level != 0: raise OpenSlideError("Invalid level") if ['fail' for s in size if s < 0]: From d821e184136f805e67490240a456a5ca505972d0 Mon Sep 17 00:00:00 2001 From: Jan Harkes Date: Fri, 17 Nov 2023 14:58:58 -0500 Subject: [PATCH 2/4] Don't allow instantiating AbstractSlide and _OpenSlideMap base classes Decorate methods with '@abstractmethod' to prevent instantiation unless all abstract methods and properties are overridden. Signed-off-by: Jan Harkes Signed-off-by: Benjamin Gilbert --- openslide/__init__.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/openslide/__init__.py b/openslide/__init__.py index 4f354b50..39bdc5ab 100644 --- a/openslide/__init__.py +++ b/openslide/__init__.py @@ -25,6 +25,7 @@ from __future__ import annotations +from abc import ABCMeta, abstractmethod from io import BytesIO from types import TracebackType from typing import Iterator, Literal, Mapping, TypeVar @@ -62,7 +63,7 @@ _T = TypeVar('_T') -class AbstractSlide: +class AbstractSlide(metaclass=ABCMeta): """The base class of a slide object.""" def __init__(self) -> None: @@ -81,22 +82,26 @@ def __exit__( return False @classmethod + @abstractmethod def detect_format(cls, filename: lowlevel.Filename) -> str | None: """Return a string describing the format of the specified file. If the file format is not recognized, return None.""" raise NotImplementedError + @abstractmethod def close(self) -> None: """Close the slide.""" raise NotImplementedError @property + @abstractmethod def level_count(self) -> int: """The number of levels in the image.""" raise NotImplementedError @property + @abstractmethod def level_dimensions(self) -> tuple[tuple[int, int], ...]: """A tuple of (width, height) tuples, one for each level of the image. @@ -109,6 +114,7 @@ def dimensions(self) -> tuple[int, int]: return self.level_dimensions[0] @property + @abstractmethod def level_downsamples(self) -> tuple[float, ...]: """A tuple of downsampling factors for each level of the image. @@ -116,6 +122,7 @@ def level_downsamples(self) -> tuple[float, ...]: raise NotImplementedError @property + @abstractmethod def properties(self) -> Mapping[str, str]: """Metadata about the image. @@ -123,6 +130,7 @@ def properties(self) -> Mapping[str, str]: raise NotImplementedError @property + @abstractmethod def associated_images(self) -> Mapping[str, Image.Image]: """Images associated with this whole-slide image. @@ -136,10 +144,12 @@ def color_profile(self) -> ImageCms.ImageCmsProfile | None: return None return ImageCms.getOpenProfile(BytesIO(self._profile)) + @abstractmethod def get_best_level_for_downsample(self, downsample: float) -> int: """Return the best level for displaying the given downsample.""" raise NotImplementedError + @abstractmethod def read_region( self, location: tuple[int, int], level: int, size: tuple[int, int] ) -> Image.Image: @@ -151,6 +161,7 @@ def read_region( size: (width, height) tuple giving the region size.""" raise NotImplementedError + @abstractmethod def set_cache(self, cache: OpenSlideCache) -> None: """Use the specified cache to store recently decoded slide tiles. @@ -299,6 +310,7 @@ def __len__(self) -> int: def __iter__(self) -> Iterator[str]: return iter(self._keys()) + @abstractmethod def _keys(self) -> list[str]: # Private method; always returns list. raise NotImplementedError() From dbd779dc4b581878c749e81e9d519e675b6899b4 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Sat, 26 Oct 2024 14:25:55 -0700 Subject: [PATCH 3/4] Make AbstractSlide.set_cache() non-abstract It was added in 1.2.0, so any older third-party subclass may not implement the method. In addition, most subclasses will want this method to do nothing, matching ImageSlide's implementation. Make the default implementation do nothing and remove the method override from ImageSlide. Signed-off-by: Benjamin Gilbert --- openslide/__init__.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/openslide/__init__.py b/openslide/__init__.py index 39bdc5ab..60a32b41 100644 --- a/openslide/__init__.py +++ b/openslide/__init__.py @@ -161,12 +161,13 @@ def read_region( size: (width, height) tuple giving the region size.""" raise NotImplementedError - @abstractmethod - def set_cache(self, cache: OpenSlideCache) -> None: + def set_cache(self, cache: OpenSlideCache) -> None: # noqa: B027 """Use the specified cache to store recently decoded slide tiles. + This class does not support caching, so this method does nothing. + cache: an OpenSlideCache object.""" - raise NotImplementedError + pass def get_thumbnail(self, size: tuple[int, int]) -> Image.Image: """Return a PIL.Image containing an RGB thumbnail of the image. @@ -486,14 +487,6 @@ def read_region( tile.info['icc_profile'] = self._profile return tile - def set_cache(self, cache: OpenSlideCache) -> None: - """Use the specified cache to store recently decoded slide tiles. - - ImageSlide does not support caching, so this method does nothing. - - cache: an OpenSlideCache object.""" - pass - def open_slide(filename: lowlevel.Filename) -> OpenSlide | ImageSlide: """Open a whole-slide or regular image. From 7d1be91f2c4afd15cafffc841e6b23326d2a7f6f Mon Sep 17 00:00:00 2001 From: Jan Harkes Date: Fri, 17 Nov 2023 15:24:28 -0500 Subject: [PATCH 4/4] lowlevel: check for errors after calling set_cache openslide_set_cache() doesn't generate any errors, but that's an implementation detail, and for consistency we should check that an error wasn't previously set. Signed-off-by: Jan Harkes Signed-off-by: Benjamin Gilbert --- openslide/lowlevel.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openslide/lowlevel.py b/openslide/lowlevel.py index 48963057..c24ca7c9 100644 --- a/openslide/lowlevel.py +++ b/openslide/lowlevel.py @@ -568,7 +568,6 @@ def read_associated_image_icc_profile( 'openslide_set_cache', None, [_OpenSlide, _OpenSlideCache], - None, minimum_version='4.0.0', )