WIP: Support fsspec mutable mapping objects in zarr.open

[maxrjones]

This is rough code, but I made some progress on supporting FSMap types and wanted to open a PR for early feedback. This isn't a priority for me, so I'd welcome anyone to take over this PR and/or close it and work in a different PR.

Addresses #2706

TODO:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/user-guide/*.rst
• Changes documented as a new file in changes/
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[maxrjones]

This is all ugly code because I don't want to assume that fsspec is installed or import it at the module level. I'll look into a better approach similar to xarray's module_available code.

[d-v-b]

could we pack this logic into a stand-alone function that basically narrows its input to a FsspecStore or errors? and maybe we could have some boolean variables in this module that represent if fsspec is present or not, e.g. something like this at the top of the file:

try:
  import fsspec
  has_fsspec = True
except ImportError:
  has_fsspec = False

[d-v-b]

could we pack this logic into a stand-alone function that basically narrows its input to a FsspecStore or errors? and maybe we could have some boolean variables in this module that represent if fsspec is present or not, e.g. something like this at the top of the file:

try:
  import fsspec
  has_fsspec = True
except ImportError:
  has_fsspec = False

[maxrjones]

I made this a function because I think it'll be helpful for wrapping sync filesystems for both from_url and from_mapper.

[d-v-b]

I recall there being issues with fsmap before, but I confess I don't really know what an fsmap is -- can someone explain what an fsmap is, how it differs from an fsspec filesystem, and why people would use one over the other?

cc @martindurant

I have a vague feeling that it could be useful to have a Store class that wraps generic MutableMapping instances, and maybe fsmaps could go there, but that requires me knowing more about the user context for fsmap.

[martindurant]

FSMap was created specifically for the needs of Zarr, and it could have been essentially the same as the v2 FSStore, but was much quicker to get out and working within dask/fsspec.

FSMap is a dict-compatible interface (mutable-mapping) on top of a FS instance, which zarr worked with since forever and ignores some FS functionality like the file-like API.

To make it work with v3 might be complex, because zarr makes async calls, and the FSMap interface is blocking, even if the underlying FS is async. That means that there will be sync() within sync(), which might still work as zarr maintains its own event loop in a thread separate from fsspec's.

[martindurant]

To be clear: this PR does not use the mapper, but constructs a normal store from the mapper's attributes. I support this path.

[maxrjones]

thanks for the questions and answers, Davis and Martin!

IIUC there are three cases that we'd need to account for:

1. FSMap wraps an async instance of an async-compatible filesystem
   Solution - as implemented in this PR, extract the wrapped filesystem and use it to open an FsspecStore
2. FSMap wraps a non-async instance of a non-async-compatible filesystem
   Solution option 1 - extract the wrapped filesystem and wrap in AsyncFileSystemWrapper to open an FsspecStore as in Wrap sync fs for xarray.to_zarr #2533
   Solution option 2 - if it's a "file" protocol, extract the wrapped filesystem and open a LocalStore rather than FsspecStore, for other protocols, wrap in AsyncFileSystemWrapper to open an FsspecStore
3. FSMap wraps a non-async instance of an async-compatibile filesystem
   Solution - this is the case I'm not sure about. Is there a way to convert from a sync instance to an async instance without needing to wrap it in AsyncFileSystemWrapper? @martindurant could you please offer guidance here?

[martindurant]

Is there a way to convert from a sync instance to an async instance without needing to wrap it in AsyncFileSystemWrapper

The instance has all the arguments it was made with as attributes, so you can make a new instance with asynchronous=True from those.

if it's a "file" protocol, extract the wrapped filesystem and open a LocalStore rather than FsspecStore

Is there a reason to bother doing this?

[maxrjones]

if it's a "file" protocol, extract the wrapped filesystem and open a LocalStore rather than FsspecStore

Is there a reason to bother doing this?

I was thinking based on https://filesystem-spec.readthedocs.io/en/latest/async.html#limitations that LocalStore may be faster since it was designed around providing an async interface.

[d-v-b]

IMO it's simpler if the path is always fsmap -> fsspecstore

[martindurant]

LocalStore may be faster since it was designed around providing an async interface.

I doubt it. The disk is not really async (at least with the standard syscalls python uses), so none of the async code should make any difference for local reads at all.