Remove path suffix check (#659)

* remove path suffix check

* remove test

* remove failing test

* add test for #593

* release note

Author: TomNicholas

docs/releases.md

@@ -49,6 +49,8 @@
   By [Tom Nicholas](https://github.com/TomNicholas).
 - Fixed bug causing coordinates to be demoted to data variables when writing to Icechunk ([#574](https://github.com/zarr-developers/VirtualiZarr/issues/574), [#588](https://github.com/zarr-developers/VirtualiZarr/pull/588))
   By [Tom Nicholas](https://github.com/TomNicholas).
+- Removed checks forbidding paths in virtual references without file suffixes ([#659](https://github.com/zarr-developers/VirtualiZarr/pull/659))
+  By [Tom Nicholas](https://github.com/TomNicholas).
 - Fixed bug when indexing a scalar ManifestArray with an ellipsis([#596](https://github.com/zarr-developers/VirtualiZarr/issues/596), [#641](https://github.com/zarr-developers/VirtualiZarr/pull/641))
   By [Max Jones](https://github.com/maxrjones) and [Tom Nicholas](https://github.com/TomNicholas).
 

virtualizarr/manifests/manifest.py

@@ -2,7 +2,6 @@
 from collections.abc import ItemsView, Iterable, Iterator, KeysView, ValuesView
 from pathlib import PosixPath
 from typing import Any, Callable, NewType, Tuple, TypedDict, cast
-from urllib.parse import urlparse, urlunparse
 
 import numpy as np
 
@@ -24,6 +23,8 @@
     "cos://",
     "minio://",
     "file:///",
+    "http://",
+    "https://",
 }
 
 
@@ -68,39 +69,13 @@ def validate_and_normalize_path_to_uri(path: str, fs_root: str | None = None) ->
     if path == "":
         # (empty paths are allowed through as they represent missing chunks)
         return path
-
-    # TODO ideally we would just use cloudpathlib.AnyPath to handle all types of paths but that would require extra dependencies, see https://github.com/drivendataorg/cloudpathlib/issues/489#issuecomment-2504725280
-
-    if path.startswith("http://") or path.startswith("https://"):
-        # hopefully would raise if given a malformed URL
-        components = urlparse(path)
-
-        if not PosixPath(components.path).suffix:
-            raise ValueError(
-                f"entries in the manifest must be paths to files, but this path has no file suffix: {path}"
-            )
-
-        return urlunparse(components)
-
     elif any(path.startswith(prefix) for prefix in VALID_URI_PREFIXES):
-        # Question: This feels fragile, is there a better way to ID a Zarr
-        if not PosixPath(path).suffix and "zarr" not in path:
-            raise ValueError(
-                f"entries in the manifest must be paths to files, but this path has no file suffix: {path}"
-            )
-
         return path  # path is already in URI form
-
     else:
         # must be a posix filesystem path (absolute or relative)
         # using PosixPath here ensures a clear error would be thrown on windows (whose paths and platform are not officially supported)
         _path = PosixPath(path)
 
-        if not _path.suffix and "zarr" not in path:
-            raise ValueError(
-                f"entries in the manifest must be paths to files, but this path has no file suffix: {path}"
-            )
-
         # only posix paths can possibly not be absolute
         if not _path.is_absolute():
             if fs_root is None:

virtualizarr/tests/test_manifests/test_manifest.py

@@ -31,13 +31,6 @@ def test_allow_http_urls(self, url):
         chunkentry = ChunkEntry.with_validation(path=url, offset=100, length=100)
         assert chunkentry["path"] == url
 
-    @pytest.mark.parametrize(
-        "path", ["/directory/file", "s3://bucket/file", "https://site.com/file"]
-    )
-    def test_disallow_paths_without_file_suffixes(self, path):
-        with pytest.raises(ValueError, match="this path has no file suffix"):
-            ChunkEntry.with_validation(path=path, offset=100, length=100)
-
     @pytest.mark.parametrize(
         "path",
         [
@@ -47,13 +40,20 @@ def test_disallow_paths_without_file_suffixes(self, path):
                     reason="cloudpathlib should ideally do stricter validation - see https://github.com/drivendataorg/cloudpathlib/issues/489"
                 ),
             ),
-            "https://site.com/###/foo.nc",
         ],
     )
     def test_catch_malformed_path(self, path):
         with pytest.raises(ValueError):
             ChunkEntry.with_validation(path=path, offset=100, length=100)
 
+    def test_allow_path_containing_colon(self):
+        # regression test for issue #593
+        ChunkEntry.with_validation(
+            path="file:///folder/wrfout_d01_2024-01-02_01:00:00.nc",
+            offset=100,
+            length=100,
+        )
+
 
 class TestConvertingRelativePathsUsingFSRoot:
     def test_fs_root_must_be_absolute(self):