Merge pull request #85 from bayer-science-for-a-better-life/refactor-…

…image-provider

Refactor image provider

paquo/images.py

@@ -1,10 +1,8 @@
 import json
 import pathlib
 import re
+import warnings
 import weakref
-from abc import ABC
-from abc import abstractmethod
-from collections.abc import Hashable
 from collections.abc import MutableMapping
 from copy import deepcopy
 from enum import Enum
@@ -43,9 +41,27 @@
 if TYPE_CHECKING:
     import paquo.projects
 
+__all__ = [
+    "ImageProvider",
+    "QuPathImageType",
+    "QuPathProjectImageEntry",
+    "SimpleFileImageId",
+]
+
 _log = get_logger(__name__)
 
 
+def __getattr__(name):
+    if name == "SimpleURIImageProvider":
+        warnings.warn(
+            "SimpleURIImageProvider is deprecated. Please use ImageProvider",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return ImageProvider
+    raise AttributeError(name)
+
+
 # [URI:java-python]
 # NOTE: pathlib handles URIs a little different to QuPath's java URIs
 #   having looked into it a little bit it seems neither are entirely
@@ -91,27 +107,53 @@ def _normalize_pathlib_uris(uri):
     return x
 
 
-class ImageProvider(ABC):
+SimpleFileImageId = Union[str, pathlib.Path]
+
+
+class ImageProvider:
     """Maps image ids to paths and paths to image ids."""
 
-    @abstractmethod
-    def uri(self, image_id: Hashable) -> Optional[str]:
-        """Returns an URI for an image given an image id tuple."""
-        # default implementation:
-        # -> null uri
-        return None
-
-    @abstractmethod
-    def id(self, uri: str) -> Hashable:
-        """Returns an image id given an URI."""
-        # default implementation:
-        # -> return filename as image id
-        return ImageProvider.path_from_uri(uri).name
-
-    @abstractmethod
+    class FilenamePathId(str):
+        """an id that uses the filename as it's identifier"""
+        def __eq__(self, other):
+            return Path(self).name == Path(other).name
+
+        def __hash__(self):
+            return hash(Path(self).name)
+
+        def __repr__(self):  # pragma: no cover
+            p = Path(self)
+            return f'FilenamePathId("{p.name}", parent="{p.parent}")'
+
+    class URIString(str):
+        """string uri's can differ in their string representation and still be identical"""
+        # we need some way to normalize uris
+        def __eq__(self, other):  # pragma: no cover
+            return ImageProvider.compare_uris(self, other)
+        __hash__ = str.__hash__  # fixme: this is not correct!
+
+    def uri(self, image_id: SimpleFileImageId) -> Optional['URIString']:
+        """accepts a path and returns a URIString"""
+        if not isinstance(image_id, (Path, str, ImageProvider.FilenamePathId)):
+            raise TypeError("image_id not of correct format")  # pragma: no cover
+        if isinstance(image_id, str) and "://" in image_id:
+            # image_id is uri
+            image_id = _normalize_pathlib_uris(image_id)
+            return ImageProvider.URIString(image_id)
+        img_path = pathlib.Path(image_id).absolute().resolve()
+        if not img_path.is_file():
+            return None
+        return ImageProvider.URIString(img_path.as_uri())
+
+    def id(self, uri: URIString) -> str:
+        """accepts a uri string and returns a FilenamePathId"""
+        if not isinstance(uri, (str, ImageProvider.URIString)):
+            raise TypeError("uri not of correct format")  # pragma: no cover
+        return ImageProvider.FilenamePathId(ImageProvider.path_from_uri(uri))
+
     def rebase(self, *uris: str, **kwargs) -> List[Optional[str]]:
-        """Allows rebasing"""
-        return [self.uri(self.id(uri)) for uri in uris]
+        uri2uri = kwargs.pop('uri2uri', {})
+        return [uri2uri.get(uri, None) for uri in uris]
 
     @staticmethod
     def path_from_uri(uri: str) -> PurePath:
@@ -168,69 +210,6 @@ def compare_uris(a: str, b: str) -> bool:
         uri_b = _normalize_pathlib_uris(b)
         return bool(uri_a.equals(uri_b))
 
-    @classmethod
-    def __subclasshook__(cls, C):
-        """ImageProviders don't need to derive but only duck-type"""
-        required_methods = ('uri', 'id', 'rebase')
-        if cls is ImageProvider:
-            methods_available = [False] * len(required_methods)
-            for B in C.__mro__:
-                for idx, method in enumerate(required_methods):
-                    methods_available[idx] |= method in B.__dict__
-                if all(methods_available):
-                    return True
-        return NotImplemented
-
-
-SimpleFileImageId = Union[str, pathlib.Path]
-
-
-# noinspection PyMethodMayBeStatic
-class SimpleURIImageProvider:
-    """simple image provider that uses the files uri as it's identifier"""
-
-    class FilenamePathId(str):
-        """an id that uses the filename as it's identifier"""
-        def __eq__(self, other):
-            return Path(self).name == Path(other).name
-
-        def __hash__(self):
-            return hash(Path(self).name)
-
-        def __repr__(self):  # pragma: no cover
-            p = Path(self)
-            return f'FilenamePathId("{p.name}", parent="{p.parent}")'
-
-    class URIString(str):
-        """string uri's can differ in their string representation and still be identical"""
-        # we need some way to normalize uris
-        def __eq__(self, other):  # pragma: no cover
-            return ImageProvider.compare_uris(self, other)
-        __hash__ = str.__hash__  # fixme: this is not correct!
-
-    def uri(self, image_id: SimpleFileImageId) -> Optional['URIString']:
-        """accepts a path and returns a URIString"""
-        if not isinstance(image_id, (Path, str, SimpleURIImageProvider.FilenamePathId)):
-            raise TypeError("image_id not of correct format")  # pragma: no cover
-        if isinstance(image_id, str) and "://" in image_id:
-            # image_id is uri
-            image_id = _normalize_pathlib_uris(image_id)
-            return SimpleURIImageProvider.URIString(image_id)
-        img_path = pathlib.Path(image_id).absolute().resolve()
-        if not img_path.is_file():
-            return None
-        return SimpleURIImageProvider.URIString(img_path.as_uri())
-
-    def id(self, uri: URIString) -> str:
-        """accepts a uri string and returns a FilenamePathId"""
-        if not isinstance(uri, (str, SimpleURIImageProvider.URIString)):
-            raise TypeError("uri not of correct format")  # pragma: no cover
-        return SimpleURIImageProvider.FilenamePathId(ImageProvider.path_from_uri(uri))
-
-    def rebase(self, *uris: str, **kwargs) -> List[Optional[str]]:
-        uri2uri = kwargs.pop('uri2uri', {})
-        return [uri2uri.get(uri, None) for uri in uris]
-
 
 # noinspection PyPep8Naming
 class _RecoveredReadOnlyImageServer:

paquo/projects.py

@@ -10,7 +10,6 @@
 from typing import Callable
 from typing import ContextManager
 from typing import Dict
-from typing import Hashable
 from typing import Iterable
 from typing import Iterator
 from typing import Optional
@@ -32,7 +31,7 @@
 from paquo.images import ImageProvider
 from paquo.images import QuPathImageType
 from paquo.images import QuPathProjectImageEntry
-from paquo.images import SimpleURIImageProvider
+from paquo.images import SimpleFileImageId
 from paquo.java import URI
 from paquo.java import BufferedImage
 from paquo.java import DefaultProject
@@ -165,7 +164,7 @@ def _stash_project_files(project_dir: pathlib.Path):
     # done
 
 
-DEFAULT_IMAGE_PROVIDER: Any = SimpleURIImageProvider()
+DEFAULT_IMAGE_PROVIDER: Any = ImageProvider()
 
 
 ProjectIOMode = Literal["r", "r+", "w", "w+", "a", "a+", "x", "x+"]
@@ -276,7 +275,7 @@ def _stage_image_entry(self, server_builder):
 
     @redirect(stderr=True, stdout=True)
     def add_image(self,
-                  image_id: Any,  # this should actually be ID type of the image provider
+                  image_id: SimpleFileImageId,
                   image_type: Optional[QuPathImageType] = None,
                   *,
                   allow_duplicates: bool = False) -> QuPathProjectImageEntry:
@@ -300,15 +299,14 @@ def add_image(self,
         img_uri = self._image_provider.uri(image_id)
         if img_uri is None:
             raise FileNotFoundError(f"image_provider can't provide URI for requested image_id: '{image_id}'")
-        img_id = self._image_provider.id(img_uri)
-        if img_id != image_id:  # pragma: no cover
-            _log.warning(f"image_provider roundtrip error: '{image_id}' -> uri -> '{img_id}'")
+        # img_id = self._image_provider.id(img_uri)
+        # if img_id != image_id:  # pragma: no cover
+        #     _log.warning(f"image_provider roundtrip error: '{image_id}' -> uri -> '{img_id}'")
 
         if not allow_duplicates:
             for entry in self.images:
-                uri = self._image_provider.id(entry.uri)
-                if img_id == uri:
-                    raise FileExistsError(img_id)
+                if img_uri == self._image_provider.uri(entry.uri):
+                    raise FileExistsError(image_id)
 
         # first get a server builder
         try:
@@ -320,7 +318,7 @@ def add_image(self,
             # it's possible that an image_provider returns an URI but that URI
             # is not actually reachable. In that case catch the java IOException
             # and raise a FileNotFoundError here
-            raise FileNotFoundError(img_uri)
+            raise FileNotFoundError(f"{image_id!r} as {img_uri!r}")
         except ExceptionInInitializerError:
             raise OSError("no preferred support found")
         if not support:
@@ -367,14 +365,16 @@ def add_image(self,
         self.save(images=False)
         return py_entry
 
-    def is_readable(self) -> Dict[Hashable, bool]:
+    def is_readable(self) -> Dict[str, bool]:
         """verify if images are reachable"""
         readability_map = {}
         for image in self.images:
-            image_id = self._image_provider.id(image.uri)
-            if image_id in readability_map:  # pragma: no cover
+            uri = image.uri
+            if uri is None:
+                raise RuntimeError(f"entry has None uri: {image!r}")
+            if uri in readability_map:  # pragma: no cover
                 raise RuntimeError("received the same image_id from image_provider for two different images")
-            readability_map[image_id] = image.is_readable()
+            readability_map[str(uri)] = image.is_readable()
         return readability_map
 
     def update_image_paths(self, *, try_relative: bool = False, **rebase_kwargs) -> None:
@@ -389,7 +389,7 @@ def update_image_paths(self, *, try_relative: bool = False, **rebase_kwargs) ->
             at a different location.
         **rebase_kwargs:
             keyword arguments are handed over to the image provider instance.
-            The default image provider is a paquo.images.SimpleURIImageProvider
+            The default image provider is a paquo.images.ImageProvider
             which uses the uri2uri keyword argument. (A mapping from old URI to new
             URI: Mapping[str, str])
 

paquo/tests/test_images.py

@@ -312,37 +312,6 @@ def test_image_provider_path_from_uri(uri: str, path: Path):
     assert c_path.parts == path.parts
 
 
-def test_image_provider_ducktyping():
-    class IPBad:
-        def id(self, x):
-            pass  # pragma: no cover
-
-    class IPGood(IPBad):  # if all required methods are implemented we're an ImageProvider
-        def uri(self, y):
-            pass  # pragma: no cover
-
-        def rebase(self, **x):
-            pass  # pragma: no cover
-
-    assert not isinstance(IPBad(), ImageProvider)
-    assert isinstance(IPGood(), ImageProvider)
-
-
-def test_image_provider_default_implementation():
-    class NoneProvider(ImageProvider):
-        def id(self, x):
-            return super().id(x)
-
-        def uri(self, y):
-            return super().uri(y)
-
-        def rebase(self, *x, **y):
-            return super().rebase(*x, **y)
-
-    ip = NoneProvider()
-    assert set(ip.rebase('file:/abc.svs', 'file:/efg.svs')) == {None}
-
-
 def test_image_provider_uri_from_relpath_and_abspath():
     with pytest.raises(ValueError):
         ImageProvider.uri_from_path(Path('./abc.svs'))