Ensure file is created when it doesnt exist

[auxym]

Resolves #3

• Change mode=rw to mode=rwc
• Add unit test

[auxym]

@newville can you please confirm this fixes the issue for you also?

[newville]

@auxym Thanks -- This also needs the if TYPE_CHECKiNG test removed, and those imports always done.

I think a basic test, with no async code, such as

import zarr
from zarr_sqlite import SQLiteStore

fname = 'test1.zarrdb'
store = SQLiteStore(fname,  read_only=False)
root = zarr.open(store=store, mode='a')
group1 = root.create_group('group1')

i = np.arange(80000)/60.0
x = i + np.random.normal(size=len(i), scale=1.5)
y = np.sin(x/13) + 0.7*np.cos(x/47) + np.random.normal(size=len(i), scale=0.05)
x.shape = (400, 200)
y.shape = (200, 400)

group1.create_array(data=x, name='xdat')
group1.create_array(data=y, name='ydat')

print(group1, list(group1.keys()))

##
print("wrote data, now reading....")
time.sleep(0.25)
read_root = zarr.open(store=SQLiteStore(fname,  read_only=True), mode='r')
ytest = read_root['group1/ydat'][()]
print(f"## DONE {read_root=} {ytest=}")

Something like this to create, write, and read should probably be included in the tests.

I find the use of async sort of odd here. I effectively never use async.

I would even say that the

read_root = zarr.open(store=SQLiteStore(fname,  read_only=True), mode='r')

is sort of a wart here. The more obvious (to me anyway)

read_root = zarr.open(store=fname, mode='r')

fails.

[auxym]

I agree on most things, will look into including your proposed test and the import fix. I also agree on async, I rarely use it, but I believe it is required to satisfy the zarr-python store abstract base class: https://github.com/zarr-developers/zarr-python/blob/main/src/zarr/abc/store.py

read_root = zarr.open(store=fname, mode='r')

I don't understand how this should work, how would zarr know it should use the SQLiteStore class to create the store based on only a file name?

[auxym]

OK, I have implemented your suggested test, see latest commit. It passes (even without the sleep).

For the imports, looking at it, I actually don't think it's necessary to remove the type_checking check, and I assume it could save time during module imports to leave it in place. You can check yourself: you can completely delete the if TYPE_CHECKING block and run the tests (uv run pytest) and every test still passes. Can you confirm that works for you also?

[newville]

TYPE_CHECKING is False at runtime (https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING).

So the code is not importing collections.abc.Iterable at runtime. But that is needed for more than type checking.
SQLiteStore.list() and SQLiteStore.list_prefix() cannot work without it.

I'll admit that I have little experience with (or love for) async, that I don't know why (from def list())

    @override
    async def list(self) -> AsyncIterator[str]:
       cur = await self._execute("SELECT k FROM zarr")
       for row in cast(Iterable[tuple[str]], cur):
            yield row[0]

is not (I do not mean "just"):

     def list(self):
        return [row[0] for row in self._execute("SELECT k FROM zarr")]

I can believe there is a reason for the extra code, and that cast(Iterable[tuple[str]], cur) is doing something useful, but I really do not know what that is. (And, like, the name is literally"list".... "returns a list" seems so obvious to me that I must be profoundly missing something and too old for all this fancy.... stuff???).

But, if you're going to use cast(Iterable[tuple[str]], cur), then, you have to import Iterable.

I would just remove the check for TYPE_CHECKING. You can time the imports -- you will find they are insignificant.

read_root = zarr.open(store=fname, mode='r')

I don't understand how this should work, how would zarr know it should use the SQLiteStore class to create the store based on only a file name?

Yeah, I guess this is a complaint about Zarr, not SQLiteStore.

root = zarr.open('store.zarr', mode='r')

works for a FileStore, but that wouldn't work for a ZipStore either. It seems like a registry of file types would be needed.

[auxym]

Oh yeah, missed those cast calls. The existing minimal tests did not test that code path. I added a whole bunch of tests (with LLM assistance) and I completely removed the if TYPE_CHECKING as you suggested.

Regarding the async stuff, my understanding is that zarr-python uses async "under the hood", but wraps a sync API around it. Thus the user of zarr can use either sync or async calls.

[newville]

@auxym Thanks.

It seems to me that the point of cast is to tell the type checker not to check types. ;) What happens if you replace for row in cast(Iterable[tuple[str]], cur): with for row in cur:? Does some IDE complain?

Regarding the async stuff, my understanding is that zarr-python uses async "under the hood", but wraps a sync API around it. Thus the user of zarr can use either sync or async calls.

I can believe all of that.

I admit a bias against async (I support a lot of Python projects for data collection and analysis, and I just never use async, even when asynchronous callbacks from a distributed control system are a vital part of the software. Threads for specific tasks, multiprocessing, yes definitely).

Async adds a bunch of keywords all over Python to bolt on coroutines that are really about asynchronous I/O (and not anything else), and then requires adding a bunch more keywords ("await"s) all over Python code to say "don't be asynchronous. You can ignore this as a rant, but I think hat

    def list(self):
        return [row[0] for row in self._execute("select k from zarr")]

has merit that

    @override
    async def list(self) -> AsyncIterator[str]:
        cur = await self._execute("SELECT k FROM zarr")
        for row in cast(Iterable[tuple[str]], cur):
            yield row[0]

does not necessarily improve. There is a bunch of code to signal non-runtime type-checkers to ignore type checking. There is code to "await" the (allegedly) expensive database lookup, and then yields over a list of values that has already been synchronously fetched. [If "cast key to str" is needed, then I think AsyncIterator[str] is not guaranteed. Perhaps the yielded value should be str(row[0])?].

When I look in detail here, it seems that all of the calls to self._execute() and self._execute_write() are awaited. Doesn't that mean these functions could be synchronous, and sort of really are synchronous?
I'll also guess that for SQLite with WAL, these database queries (simple select in a two-column table) will be fast.
I'm comfortable saying that any claim about speed needs real benchmarks, but I'll say that goes for the claim that "using async makes stuff faster".

Similarly, where the code has await self._ensure_open(), couldn't that be if not self.is_open: open()?

Feel free to ignore all this.

[auxym]

Yes, I believe the casts were necessary to stop mypy/pyright from complaining.

That said,

Perhaps the yielded value should be str(row[0])?].

I do like that suggestion. Latest commit got rid of all casts :)

When I look in detail here, it seems that all of the calls to self._execute() and self._execute_write()

Yeah, I wrapped all db transactions in those methods. Yes, they are currently blocking (non-async), since the stdlib sqlite3 is blocking. The goal of putting all transactions in a central place is that I aim, in the future, to implement real async transactions, either using aiosqlite3 (3rd party lib) or implementing our own background thread for db work (probably the former). I also personally do not use async, but zarr itself offers async everywhere, so it would be a better match for that pattern.

Similarly, where the code has await self._ensure_open(), couldn't that be if not self.is_open: open()?

_ensure_open is from zarr's store base class, once again I'm trying to follow zarr-python's existing patterns.

https://github.com/zarr-developers/zarr-python/blob/50b7e016590d76011382771fd52843949782c9c0/src/zarr/abc/store.py#L140