Repository navigation
Add store.with_read_only() copy method #3105
Description
Thank you for the efforts that have gone into hardening Zarr-Python's approach to read/write modes!
Now that it's no longer possible to open an array in read mode with a store with read_only=False, I'm wondering if it would be possible/advisable to add a public setter method to modify a store's mode? This is the code that I had been working with. It'd be convenient to open a read only array without needing to create a new store instance:
def array_roundtrip(store):
"""
Round trip an array using a Zarr store
Args:
store: Store-Like object
"""
data = np.ones((3,3))
arr = zarr.create_array(store=store, overwrite=True, data=data)
assert isinstance(arr, Array)
# Read set values
arr2 = zarr.open_array(store=store, mode="r")
assert isinstance(arr2, Array)
np.testing.assert_array_equal(arr[:], data)
Activity
d-v-b commentedon May 30, 2025
I'm pretty strongly opposed to setting attributes of an object after its been created, but another way to achieve this is to create a brand new store with a different mode. An idea I had a while ago was that each store would have a
with_modemethod that would return a new store instance with a different mode.related: #2448
d-v-b commentedon May 30, 2025
@TomAugspurger added
with_modeto the store abc here, but I'm not seeing that method any more. Now I'm curious when it was removed...maxrjones commentedon May 30, 2025
FYI I ran into some confusing behavior with
xr.open_zarrin virtual_tiff returning an empty dataset when using Zarr-Python's main branch, which didn't happen during the latest release. This led me to run xarray's upstream tests locally which showed many failures related to the new mode handling. I'm not sure yet if this is related to the issues in my virtual_tiff tests, but wanted to share that the change as-is has a high disruptive potential. (cc @jhamman)d-v-b commentedon May 30, 2025
likely due to changes added in #3068
dstansby commentedon May 30, 2025
Why so?
I'd be 👍 to adding this functionality - one case I can think of where one wouldn't want to copy the store would be creating a memory store, filling it with some data, then marking it as read only.
maxrjones commentedon May 30, 2025
The following shows why a setter is actually a bad idea unless array and store modes are decoupled, since it's not obvious to users that array mode state is deferred to the store. So someone could change the store mode to change the contents of one array / group and not expect that it changes the write mode of all arrays / groups linked to that store.
A convenience function to create a copy with a different mode could be nice though.
d-v-b commentedon May 31, 2025
In the normal use of a memory store it just holds a reference to a python dict, which itself is mutable no matter what because of python dict semantics. So copying the store doesn't actually copy any data, which makes it very cheap to do.
As for why I don't like a mutable attribute for this kind of thing, IMO it becomes very hard to reason about the read / write semantics of an object if those semantics can change during the object's lifecycle. It's much easier to reason about read / writeness if that property is immutable and fixed when creating the store instance.
dstansby commentedon May 31, 2025
👍 - I didn't realise it was easy to copy a memory store without copying the contents.
dstansby commentedon May 31, 2025
In which case, it seems like a good way forward here is to implement some kind of
with_mode()method?d-v-b commentedon May 31, 2025
i think that's a good way forward. for every store except zipstore this will work easily, and for zipstore we can raise
NotImplementedError[-]Add a store.read_only setter method[/-][+]Add store.with_mode() opening method[/+][-]Add store.with_mode() opening method[/-][+]Add store.with_mode() constructor method[/+][-]Add store.with_mode() constructor method[/-][+]Add store.with_read_only() copy method[/+]TomNicholas commentedon Jun 20, 2025
This is definitely disrupting me - I'm trying to develop using a PR branched off xarray
main, and this PR to zarr, but I see the same error @maxrjones did:This also causes at least one failure in the xarray upstream test suite in CI.
We obviously need to fix this before anyone releases, but even before then I there a way I can work around this to continue local development? e.g. is it fixed in zarr
mainyet?Note that weirdly using the custom
LatencyStoreI defined for my xarray test doesn't produce this problem, for reasons I don't understand.d-v-b commentedon Jun 20, 2025
your latency store has immunity because read-only is hard-coded to
True:)TomNicholas commentedon Jun 20, 2025
I should have realised that haha. Cool thanks, that helps me work around this in a minimal way until there is a fix.
trim latencystore down to just what's needed to dodge zarr-developers…
d-v-b commentedon Jun 26, 2025
@maxrjones is this resolved now that #3138 is merged?
Add asynchronous load method (#10327)