fix consolidate_metadata with FSStore

[jhamman]

This PR fixes a small bug in the consolidate_metadata convenience function where, when using fsspec compatible store paths, the FSStore was opened as read-only, causing the write of the consolidated metadata key to fail.

Closes #915

cc @martindurant, @freeman-lab

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/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

+1 for the idea

I wonder if we should check that store is a string before doing this - might we otherwise end up writing to a store that has been opened explicitly as readonly? I'm not sure if that's a side-effect, maybe this comment can be ignored.

[codecov]

Codecov Report

Merging #916 (fd8ca6f) into master (5f7e2b6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #916   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          32       32           
  Lines       11216    11219    +3     
=======================================
+ Hits        11210    11213    +3     
  Misses          6        6           

Impacted Files	Coverage Δ
zarr/convenience.py	100.00% <100.00%> (ø)
zarr/tests/test_storage.py	100.00% <100.00%> (ø)

[jhamman]

@martindurant -

I wonder if we should check that store is a string before doing this - might we otherwise end up writing to a store that has been opened explicitly as readonly? I'm not sure if that's a side-effect, maybe this comment can be ignored.

I think this is already take care of in normalize_store_arg:

zarr-python/zarr/storage.py

Lines 112 to 115 in 4fb0f7e

	if isinstance(store, str):
	mode = mode if clobber else "r"
	if "://" in store or "::" in store:
	return FSStore(store, mode=mode, **(storage_options or {}))

[joshmoore]

Thanks, @jhamman! This could fix or at least play a role in fixing:

• Bug: zarr.open behaves different than zarr.open_group with mode w- on gs:// URIs #712
• Read only array default upon opening #809

[jhamman]

Thanks, @jhamman! This could fix or at least play a role in fixing:

• Bug: zarr.open behaves different than zarr.open_group with mode w- on gs:// URIs #712
• Read only array default upon opening #809

It's quite possible these are related, but I think the relation is, at most, limited to how FSStore is instantiated in various places. The issue (and changes here) are confined to consolidate_metadata and the clobber argument to normalize_store_arg. It may be worth taking a deeper look at the call path for normalize_store_arg to see if there is some relation.