Merge pull request #80 from bayer-science-for-a-better-life/fix-windo…

…ws-network-share

Fix windows network share URIs in paquo

.pre-commit-config.yaml

@@ -6,7 +6,7 @@ repos:
   - id: end-of-file-fixer
   - id: check-added-large-files
 - repo: https://github.com/asottile/pyupgrade
-  rev: v2.37.2
+  rev: v3.2.2
   hooks:
   - id: pyupgrade
     args: [--py37-plus]
@@ -21,13 +21,13 @@ repos:
 #   - id: black
 #     language_version: python3
 - repo: https://github.com/pre-commit/mirrors-mypy
-  rev: 'v0.971'
+  rev: 'v0.990'
   hooks:
   - id: mypy
     exclude: ^examples|^extras|^docs|tests.*
     additional_dependencies: [packaging, ome-types]
 - repo: https://github.com/PyCQA/flake8
-  rev: '4.0.1'
+  rev: '5.0.4'
   hooks:
   - id: flake8
     additional_dependencies:
@@ -38,4 +38,4 @@ repos:
   rev: '1.7.4'
   hooks:
   - id: bandit
-    args: ["--ini", ".bandit"]
+    args: ["-lll", "--ini", ".bandit"]

CONTRIBUTING.md

@@ -45,7 +45,7 @@ where N is the issue number.
 ## Merge approval
 
 Repository maintainers will review the pull request and make sure it provides
-the appropriate level of code quality & correctness. 
+the appropriate level of code quality & correctness.
 
 
 ## How are decisions made?

paquo/images.py

@@ -19,6 +19,9 @@
 from typing import List
 from typing import Optional
 from typing import Union
+from urllib.parse import quote
+from urllib.parse import urlsplit
+from urllib.parse import urlunsplit
 
 from paquo._logging import get_logger
 from paquo._logging import redirect
@@ -53,16 +56,39 @@
 def _normalize_pathlib_uris(uri):
     """this will correctly unescape and normalize uri's received from pathlib.Path.as_uri()"""
     # https://docs.oracle.com/javase/7/docs/api/java/net/URI.html section Identities
-    u = URI(uri)
-    return URI(
-        u.getScheme(),
-        u.getUserInfo(),
-        u.getHost(),
-        u.getPort(),
-        u.getPath(),
-        u.getQuery(),
-        u.getFragment()
-    )
+    try:
+        u = URI(uri)
+    except URISyntaxException:
+        try:
+            s = urlsplit(uri)
+            s = s._replace(path=quote(s.path))
+            uri = urlunsplit(s)
+        except ValueError:
+            raise ValueError(f"uri not valid '{uri}'")
+        else:
+            u = URI(uri)
+    scheme = u.getScheme()
+    if scheme != "file":
+        raise ValueError(f"uri unsupported scheme '{uri}'")
+    host = u.getHost()
+    path = str(u.getPath())
+    if host:
+        path = f"////{host}{path}"
+    elif re.match("^//[^/]+/[a-zA-Z][$]/", path):
+        path = f"//{path}"
+    try:
+        x = URI(
+            scheme,
+            u.getUserInfo(),
+            None,
+            u.getPort(),
+            path,
+            u.getQuery(),
+            u.getFragment()
+        )
+    except URISyntaxException:
+        raise ValueError(f"uri syntax error '{uri}'")
+    return x
 
 
 class ImageProvider(ABC):
@@ -93,15 +119,17 @@ def path_from_uri(uri: str) -> PurePath:
         Parses an URI representing a file system path into a Path.
         """
         # TODO: needs way more tests... See note [URI:java-python]
-        try:
-            java_uri = URI(uri)
-        except URISyntaxException:
-            raise ValueError(f"not a valid uri '{uri}'")
+        java_uri = _normalize_pathlib_uris(uri)
         # test current scheme support
         if str(java_uri.getScheme()) != "file":
             raise NotImplementedError("paquo only supports file:/ URIs as of now")
         else:
             path_str = str(java_uri.getPath())
+
+        host = java_uri.getHost()
+        if host:
+            path_str = f"//{host}{path_str}"
+
         # fixme: this should be replaced with something more reliable...
         # check if we encode a windows path
         if re.match(r"/[A-Z]:/[^/]", path_str):
@@ -184,6 +212,10 @@ 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

paquo/jpype_backend.py

@@ -33,12 +33,12 @@
 
 
 def find_qupath(*,
-                qupath_dir: PathOrStr = None,
-                qupath_search_dirs: Union[PathOrStr, List[PathOrStr]] = None,
-                qupath_search_dir_regex: str = None,
-                qupath_search_conda: bool = None,
-                qupath_prefer_conda: bool = None,
-                java_opts: Union[List[str], str] = None,
+                qupath_dir: Optional[PathOrStr] = None,
+                qupath_search_dirs: Optional[Union[PathOrStr, List[PathOrStr]]] = None,
+                qupath_search_dir_regex: Optional[str] = None,
+                qupath_search_conda: Optional[bool] = None,
+                qupath_prefer_conda: Optional[bool] = None,
+                java_opts: Optional[Union[List[str], str]] = None,
                 **_kwargs) -> QuPathJVMInfo:
     """find current qupath installation and jvm paths/options
 

paquo/projects.py

@@ -301,9 +301,8 @@ def add_image(self,
         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 not img_id == image_id:  # pragma: no cover
+        if img_id != image_id:  # pragma: no cover
             _log.warning(f"image_provider roundtrip error: '{image_id}' -> uri -> '{img_id}'")
-            raise RuntimeError("the image provider failed to roundtrip the image id correctly")
 
         if not allow_duplicates:
             for entry in self.images:
@@ -315,13 +314,13 @@ def add_image(self,
         try:
             support = ImageServerProvider.getPreferredUriImageSupport(
                 BufferedImage,
-                String(str(img_uri))
+                String(img_uri),
             )
         except IOException:  # pragma: no cover
             # 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(image_id)
+            raise FileNotFoundError(img_uri)
         except ExceptionInInitializerError:
             raise OSError("no preferred support found")
         if not support:

paquo/tests/conftest.py

@@ -40,6 +40,14 @@ def svs_small():
         yield img_fn.absolute()
 
 
+@pytest.fixture(scope="function")
+def renamed_svs_small(request, tmp_path, svs_small):
+    name = request.param
+    new_path = tmp_path.joinpath(name)
+    new_path.write_bytes(svs_small.read_bytes())
+    yield new_path
+
+
 @pytest.fixture(scope='session')
 def qupath_version():
     from paquo.java import qupath_version

paquo/tests/test_projects.py

@@ -1,5 +1,6 @@
 import os
 import platform
+import posixpath
 import shutil
 import tempfile
 from contextlib import nullcontext
@@ -179,6 +180,33 @@ def test_project_add_image(new_project, svs_small):
     assert object() not in new_project.images
 
 
+@pytest.mark.skipif(platform.system() != "Windows", reason="windows only")
+def test_project_add_image_windows(new_project, svs_small):
+    drive, *parts = svs_small.parts
+
+    uri = posixpath.join(f"file:////localhost/{drive[0].lower()}$", *parts)
+
+    _ = new_project.add_image(uri)
+    assert len(new_project.images) == 1
+
+
+@pytest.mark.skipif(platform.system() != "Windows", reason="windows only")
+@pytest.mark.parametrize(
+    "renamed_svs_small", [
+        pytest.param("abc.svs", id="simple_name"),
+        pytest.param("image%image.svs", id="name_with_percentage"),
+    ],
+    indirect=True,
+)
+def test_project_add_image_windows_unencoded_uri(new_project, renamed_svs_small):
+    drive, *parts = renamed_svs_small.parts
+
+    uri = posixpath.join(f"file:////localhost/{drive[0]}$", *parts)
+
+    _ = new_project.add_image(uri)
+    assert len(new_project.images) == 1
+
+
 def test_project_add_image_writes_project(tmp_path, svs_small):
     qp = QuPathProject(tmp_path, mode='x')
     qp.add_image(svs_small)
@@ -258,7 +286,7 @@ def test_project_delete_image_file_when_opened(new_project, svs_small):
 
     if qupath_uses == "BIOFORMATS":  # pragma: no cover
         if platform.system() == "Windows":
-            # NOTE: on windows because you can't delete files that have open
+            # NOTE: on Windows because you can't delete files that have open
             #   file handles. In this test we're deleting the file opened by
             #   the ImageServer on the java side. (this is happening
             #   implicitly when calling is_readable() because the java