Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions changes/4533.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Mimic fsspec behavior when parsing file:// URLs

File URLs like

`file://a`

were being parsed in `parse_store_url` with a path like

`ParsedStoreUrl(..., path="")`

instead of

`ParsedStoreUrl(..., path="./a")`

which is the behavior in `fsspec`. This also makes resolves the parsed
paths to their absolute representation immediately, to prevent eventual
changes in CWD from affecting the resolution of the path later.
26 changes: 17 additions & 9 deletions src/zarr/storage/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,24 @@ def parse_store_url(url: str) -> ParsedStoreUrl:

parsed = urlparse(url)

# netloc is the "host" part (store name for memory://, bucket for s3://, etc.)
name = parsed.netloc or None

# For URLs with a scheme and netloc (like memory://store/path or s3://bucket/key),
# strip the leading slash from the path component.
# For local paths (no scheme), preserve the path as-is.
if parsed.scheme and parsed.netloc:
path = parsed.path.lstrip("/")
# Since we're allowing windows paths to carry URL separator characters,
# we'll do the same for posix paths as well
if parsed.scheme == "":
abs_path = Path(url).absolute()
return ParsedStoreUrl(scheme="", name=None, path=str(abs_path), raw=url)

if parsed.scheme == "file":
if parsed.query:
raise ValueError(f"Query is not supported in file:// URLs: {parsed.query}")
if parsed.fragment:
raise ValueError(f"Fragments are not supported in file:// URLs: {parsed.fragment}")
name = None
# mimic fsspec and interpret file://a/b as ./a/b
path = str(Path(parsed.netloc + parsed.path).absolute())
else:
path = parsed.path
# netloc is the "host" part (store name for memory://, bucket for s3://, etc.)
name = parsed.netloc or None
path = parsed.path.lstrip("/")

return ParsedStoreUrl(scheme=parsed.scheme, name=name, path=path, raw=url)

Expand Down
56 changes: 51 additions & 5 deletions tests/test_store/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import sys
from typing import TYPE_CHECKING
from unittest.mock import patch

import pytest
Expand All @@ -9,6 +10,9 @@
from zarr.core.buffer.core import default_buffer_prototype
from zarr.storage._utils import ParsedStoreUrl, _normalize_byte_range_index, parse_store_url

if TYPE_CHECKING:
from pathlib import Path


class TestParseStoreUrl:
"""Tests for parse_store_url."""
Expand Down Expand Up @@ -39,18 +43,60 @@ def test_s3_url(self) -> None:
scheme="s3", name="bucket", path="key", raw="s3://bucket/key"
)

def test_file_url(self) -> None:
result = parse_store_url("file:///tmp/test")
assert result.scheme == "file"
@pytest.mark.parametrize(
("url", "expected_path"),
[
("file:///tmp/test", "/tmp/test"),
("file:///a/b", "/a/b"),
("file:///a/b/", "/a/b"),
("file:///", "/"),
],
)
def test_absolute_file_url(self, url: str, expected_path: str) -> None:
"""file:///a/b (empty netloc) is an absolute path."""
result = parse_store_url(url)
assert result == ParsedStoreUrl(scheme="file", name=None, path=expected_path, raw=url)

@pytest.mark.parametrize(
("url", "relative_path"),
[
("file://a/b", "a/b"),
("file://a/b/", "a/b"),
("file://a", "a"),
("file://./a/b", "a/b"),
("file://", ""),
],
)
def test_relative_file_url(
self, url: str, relative_path: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""file://a/b is a path relative to the cwd, not a host named "a"."""
monkeypatch.chdir(tmp_path)
result = parse_store_url(url)
assert result == ParsedStoreUrl(
scheme="file", name=None, path=str(tmp_path / relative_path), raw=url
)

@pytest.mark.parametrize(
"url",
[
"file:///a/b?query=1",
"file:///a/b#fragment",
],
)
def test_file_url_rejects_params_query_fragment(self, url: str) -> None:
with pytest.raises(ValueError, match="not supported in file:// URLs"):
parse_store_url(url)

def test_local_absolute_path(self) -> None:
result = parse_store_url("/local/path")
assert result == ParsedStoreUrl(scheme="", name=None, path="/local/path", raw="/local/path")

def test_local_relative_path(self) -> None:
def test_local_relative_path(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.chdir(tmp_path)
result = parse_store_url("relative/path")
assert result == ParsedStoreUrl(
scheme="", name=None, path="relative/path", raw="relative/path"
scheme="", name=None, path=str(tmp_path / "relative/path"), raw="relative/path"
)

@pytest.mark.parametrize(
Expand Down
Loading