Require auto_mkdir for LocalFileSystem

Author: maxrjones

src/zarr/storage/_fsspec.py

@@ -47,6 +47,12 @@ def _make_async(fs: AbstractFileSystem) -> AsyncFileSystem:
         fs_dict = fs.to_dict()
         fs_dict["asynchronous"] = True
         return AbstractFileSystem.from_dict(fs_dict)
+    from fsspec.implementations.local import LocalFileSystem
+
+    if type(fs) is LocalFileSystem and not fs.auto_mkdir:
+        raise ValueError(
+            f"LocalFilesystem {fs} was created with auto_mkdir=False but Zarr requires the filesystem to automatically create directories"
+        )
     try:
         from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper
 

tests/test_store/test_fsspec.py

@@ -108,35 +108,94 @@ async def test_basic() -> None:
     assert out[0].to_bytes() == data[1:]
 
 
-@pytest.mark.xfail(reason="See https://github.com/zarr-developers/zarr-python/issues/2808")
-def test_open_fsmap_file(tmp_path: pathlib.Path) -> None:
-    fsspec = pytest.importorskip("fsspec")
-    fs = fsspec.filesystem("file")
-    mapper = fs.get_mapper(tmp_path)
-    arr = zarr.open(store=mapper, mode="w", shape=(3, 3))
+def array_roundtrip(store):
+    """
+    Round trip an array using a Zarr store
+
+    Args:
+        store: Store-Like object (e.g., FSMap)
+    """
+    arr = zarr.open(store=store, mode="w", shape=(3, 3))
     assert isinstance(arr, Array)
     # Set values
     arr[:] = 1
     # Read set values
-    arr = zarr.open(store=mapper, mode="r", shape=(3, 3))
+    arr = zarr.open(store=store, mode="r", shape=(3, 3))
     assert isinstance(arr, Array)
     np.testing.assert_array_equal(np.ones((3, 3)), arr[:])
 
 
+@pytest.mark.skipif(
+    parse_version(fsspec.__version__) < parse_version("2024.12.0"),
+    reason="No AsyncFileSystemWrapper",
+)
+def test_wrap_sync_filesystem(tmp_path):
+    """The local fs is not async so we should expect it to be wrapped automatically"""
+    from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper
+
+    store = FsspecStore.from_url(f"local://{tmp_path}", storage_options={"auto_mkdir": True})
+    assert isinstance(store.fs, AsyncFileSystemWrapper)
+    assert store.fs.async_impl
+    array_roundtrip(store)
+
+
+@pytest.mark.skipif(
+    parse_version(fsspec.__version__) >= parse_version("2024.12.0"),
+    reason="No AsyncFileSystemWrapper",
+)
+def test_wrap_sync_filesystem_raises(tmp_path):
+    """The local fs is not async so we should expect it to be wrapped automatically"""
+    with pytest.raises(ImportError, match="The filesystem .*"):
+        FsspecStore.from_url(f"local://{tmp_path}", storage_options={"auto_mkdir": True})
+
+
+@pytest.mark.skipif(
+    parse_version(fsspec.__version__) < parse_version("2024.12.0"),
+    reason="No AsyncFileSystemWrapper",
+)
+def test_no_wrap_async_filesystem():
+    """An async fs should not be wrapped automatically; fsspec's s3 filesystem is such an fs"""
+    from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper
+
+    store = FsspecStore.from_url(
+        f"s3://{test_bucket_name}/foo/spam/",
+        storage_options={"endpoint_url": endpoint_url, "anon": False},
+    )
+    assert not isinstance(store.fs, AsyncFileSystemWrapper)
+    assert store.fs.async_impl
+    array_roundtrip(store)
+
+
+@pytest.mark.skipif(
+    parse_version(fsspec.__version__) < parse_version("2024.12.0"),
+    reason="No AsyncFileSystemWrapper",
+)
+def test_open_fsmap_file(tmp_path: pathlib.Path) -> None:
+    fsspec = pytest.importorskip("fsspec")
+    fs = fsspec.filesystem("file", auto_mkdir=True)
+    mapper = fs.get_mapper(tmp_path)
+    array_roundtrip(mapper)
+
+
+@pytest.mark.skipif(
+    parse_version(fsspec.__version__) < parse_version("2024.12.0"),
+    reason="No AsyncFileSystemWrapper",
+)
+def test_open_fsmap_file_raises(tmp_path: pathlib.Path) -> None:
+    fsspec = pytest.importorskip("fsspec.implementations.local")
+    fs = fsspec.LocalFileSystem(auto_mkdir=False)
+    mapper = fs.get_mapper(tmp_path)
+    with pytest.raises(ValueError, match="LocalFilesystem .*"):
+        array_roundtrip(mapper)
+
+
 @pytest.mark.parametrize("asynchronous", [True, False])
 def test_open_fsmap_s3(asynchronous: bool) -> None:
     s3_filesystem = s3fs.S3FileSystem(
         asynchronous=asynchronous, endpoint_url=endpoint_url, anon=False
     )
     mapper = s3_filesystem.get_mapper(f"s3://{test_bucket_name}/map/foo/")
-    arr = zarr.open(store=mapper, mode="w", shape=(3, 3))
-    assert isinstance(arr, Array)
-    # Set values
-    arr[:] = 1
-    # Read set values
-    arr = zarr.open(store=mapper, mode="r", shape=(3, 3))
-    assert isinstance(arr, Array)
-    np.testing.assert_array_equal(np.ones((3, 3)), arr[:])
+    array_roundtrip(mapper)
 
 
 def test_open_s3map_raises() -> None:
@@ -147,12 +206,12 @@ def test_open_s3map_raises() -> None:
     with pytest.raises(
         ValueError, match="'path' was provided but is not used for FSMap store_like objects"
     ):
-        zarr.open(store=mapper, mode="w", shape=(3, 3), path="foo")
+        zarr.open(store=mapper, path="bar", mode="w", shape=(3, 3))
     with pytest.raises(
         ValueError,
         match="'storage_options was provided but is not used for FSMap store_like objects",
     ):
-        zarr.open(store=mapper, mode="w", shape=(3, 3), storage_options={"anon": True})
+        zarr.open(store=mapper, storage_options={"anon": True}, mode="w", shape=(3, 3))
 
 
 @pytest.mark.parametrize("asynchronous", [True, False])
@@ -276,31 +335,3 @@ async def test_empty_nonexistent_path(self, store_kwargs) -> None:
         store_kwargs["path"] += "/abc"
         store = await self.store_cls.open(**store_kwargs)
         assert await store.is_empty("")
-
-
-@pytest.mark.skipif(
-    parse_version(fsspec.__version__) < parse_version("2024.12.0"),
-    reason="No AsyncFileSystemWrapper",
-)
-def test_wrap_sync_filesystem():
-    """The local fs is not async so we should expect it to be wrapped automatically"""
-    from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper
-
-    store = FsspecStore.from_url("local://test/path")
-
-    assert isinstance(store.fs, AsyncFileSystemWrapper)
-    assert store.fs.async_impl
-
-
-@pytest.mark.skipif(
-    parse_version(fsspec.__version__) < parse_version("2024.12.0"),
-    reason="No AsyncFileSystemWrapper",
-)
-def test_no_wrap_async_filesystem():
-    """An async fs should not be wrapped automatically; fsspec's https filesystem is such an fs"""
-    from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper
-
-    store = FsspecStore.from_url("https://test/path")
-
-    assert not isinstance(store.fs, AsyncFileSystemWrapper)
-    assert store.fs.async_impl