Support init of ome_zarr_py.io.ZarrLocation with zarr.storage.FSStore

[berombau]

By supporting FSStore as input, more complex ZarrLocations are possible like private remote stores.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.36%. Comparing base (f79bad0) to head (6d718e0).

❗ Current head 6d718e0 differs from pull request most recent head 03d8f53. Consider uploading reports for the commit 03d8f53 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #349      +/-   ##
==========================================
+ Coverage   85.06%   85.36%   +0.30%     
==========================================
  Files          13       13              
  Lines        1533     1531       -2     
==========================================
+ Hits         1304     1307       +3     
+ Misses        229      224       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joshmoore]

Thanks, @berombau. Would you be up for adding a test that makes use of this?

[berombau]

@joshmoore Sure, I added some basic tests. More bespoke testing would require a dependency on s3fs or universal_pathlib, but this covers my use case.

[joshmoore]

Hey @berombau. Sorry for the slow response.

Sure, I added some basic tests.

❤️

More bespoke testing would require a dependency on s3fs or universal_pathlib

fsspec[s3] does pull in s3fs if you want to add it into the testing. I also don't think there would be any objections to a dependency on universal_pathlib. A step further might be to add moto for testing like in https://github.com/zarr-developers/zarr-python/blob/a81db0782535ba04c32c277102a6457d118a73e8/zarr/tests/test_storage.py#L1380

The changes all look good to me, and I can see getting them in as they are. The one additional step that we likely want to take as a follow-up is to better specify what's required of the FSStore instance that is to be used, i.e., to better document the requirements of the init_store() method both for anyone wanting to pass their own FSStore but also for anyone subclassing that method.

Hopefully with the next major version we will be dropping many of the requirements on that method, so that implementations other than FSStore (and with arbitrary dimension separators) can be supported.

cc: @will-moore

[pgarrison]

This PR would solve #369 for bioio-ome-zarr. What needs to happen to get this change merged?

[berombau]

It's ready to go in the next release, I think. I worked with universal-pathlib and moto in a spatialdata fork and it works well, but it's better to work on that if needed in a separate PR.

[joshmoore]

Thanks, all! Due to the change of the API, going to release this now as 0.9.0. @pgarrison, interested to hear if it solves all your troubles. 🙂

[pgarrison]

@joshmoore yes I was able to use this pattern to read a public s3:// path without authentication, thanks all!