-
-
Notifications
You must be signed in to change notification settings - Fork 331
Add a store.read_only setter method #3105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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 related: #2448 |
@TomAugspurger added |
FYI I ran into some confusing behavior with
|
likely due to changes added in #3068 |
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. |
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. import zarr
import zarr.storage
import numpy as np
store = zarr.storage.LocalStore("test-dir", read_only=False)
read_only_store = zarr.storage.LocalStore("test-dir", read_only=True)
arr = zarr.array(store=store, data=np.ones((3,3)))
arr = zarr.open_array(read_only_store, mode="r")
print(arr[:])
# Change my read only store to be writable
read_only_store._read_only = False
# Write to the array that I opened as read only
arr[:] = 42
print(arr[:])
arr A convenience function to create a copy with a different mode could be nice though. |
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. |
👍 - I didn't realise it was easy to copy a memory store without copying the contents. |
In which case, it seems like a good way forward here is to implement some kind of |
i think that's a good way forward. for every store except zipstore this will work easily, and for zipstore we can raise |
Uh oh!
There was an error while loading. Please reload this page.
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:The text was updated successfully, but these errors were encountered: