Removing Tensorstore Dependency

[Lencripe]

Tensorstore not required after the zarr update.

[Lencripe]

@DragaDoncila Please review.

[DragaDoncila]

Functionality works as expected which is great. It looks like you've left your demo widget in there and I'm not quite sure it's ready for contribution 😛

Only other thing is we should remove the tensorstore dependency in setup.cfg if this is the only place we were using it!

[DragaDoncila]

Also @jni you probs want to review (and approve workflow 😆 )

[jni]

This should be reformatted to follow PEP8. You can use pip install pre-commit and then pre-commit install in the root directory so that yapf is run before each commit and the code is always properly formatted.

[Lencripe]

Thanks, Juan I'll make sure to do sore in future commits :) .

[Lencripe]

Hi Juan
I've been getting this error every time I try make a commit with pre-commit do you have any idea what the cause could be?
` An unexpected error has occurred: OperationalError: unable to open database file
Failed to write to log at /Users/lencrpe/.cache/pre-commit/pre-commit.log

version information

pre-commit version: 2.20.0
git --version: git version 2.38.1
sys.version:
    3.9.13 | packaged by conda-forge | (main, May 27 2022, 17:00:52) 
    [Clang 13.0.1 ]
sys.executable: /Users/lencrpe/opt/anaconda3/envs/napari-env/bin/python
os.name: posix
sys.platform: darwin

error information

An unexpected error has occurred: OperationalError: unable to open database file

Traceback (most recent call last):
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/error_handler.py", line 73, in error_handler
    yield
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/main.py", line 386, in main
    return run(args.config, store, args)
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/commands/run.py", line 411, in run
    for hook in all_hooks(config, store)
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/repository.py", line 227, in all_hooks
    return tuple(
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/repository.py", line 230, in <genexpr>
    for hook in _repository_hooks(repo, store, root_config)
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/repository.py", line 205, in _repository_hooks
    return _cloned_repository_hooks(repo_config, store, root_config)
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/repository.py", line 171, in _cloned_repository_hooks
    manifest_path = os.path.join(store.clone(repo, rev), C.MANIFEST_FILE)
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/store.py", line 186, in clone
    return self._new_repo(repo, ref, deps, clone_strategy)
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/store.py", line 130, in _new_repo
    result = _get_result()
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/store.py", line 123, in _get_result
    with self.connect() as db:
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/contextlib.py", line 119, in __enter__
    return next(self.gen)
  File "/Users/lencrpe/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/pre_commit/store.py", line 100, in connect
    with contextlib.closing(sqlite3.connect(db_path)) as db:
sqlite3.OperationalError: unable to open database file

`

[jni]

Wow that is super weird! Not only have I never seen that but I also can't find any references to it in the context of pre-commit! My only (weak) suggestion is to try a brand new environment? 🤞

[jni]

I don't love that napari.yaml got reformatted too... Do you know what tool did that? (It shouldn't be yapf but I don't actually know.)

[jni]

Sorry, wrong button 😅

The tests are failing because you need to move the tensorstore dependency from install_requires to options.extras_require / testing, rather than deleting it. After that I think this is ready to go!

[Lencripe]

Hi Juan
I initially tried to change the syntax of the code using my IDE however I later realised it wasn't using the yapf formatter so that may explain the changes made to the napery.yaml file. I have however reverted those changes by checking out the napari.yaml from an earlier commit.

I spent sometime yesterday reading through pre-commit's documentation and I finally managed to fix my weird error 🥳which as it turns out was not related to pre-commit at all 😂 however the information I learnt should be useful when I start working on the pre-commit & GitHub actions for Napari-graph.I've also moved tensorstore dependency to testing so hopefully the tests pass this time 🤞🏾.

[jni]

Hmmm, ok, so it looks like there's more changes to be made. Sorry, I am not very familiar with tox, which is used by the cookiecutter template for plugins to set up all of the testing configurations. So right now it only installs the dependencies, not the testing dependencies. That's a problem! 😅 In order to get the extras, you can add an extras = testing to tox.ini, as done in napari here:

https://github.com/napari/napari/blob/c871c319247b79d4c5cc9270c0fec04740aed9d0/tox.ini#L79-L83

Another issue is that the solution isn't quite what we're after: we don't want to have open_tensorstore not use tensorstore. What we want is the following:

• add an open_zarr function that does the same thing as open tensorstore, but with zarr.
• expose that in the manifest. You can use slightly modified display names, e.g. "Open zarr file (tensorstore)" vs "Open zarr file (zarr)".
• in the actual code, instead of doing import tensorstore at the top of the file, which will fail if a user doesn't have tensorstore installed, move the import inside the open_tensorstore function, using this pattern:

try:
    import tensorstore
except ModuleNotFoundError as e:
    msg = 'You must install the optional tensorstore library to open a zarr file with it.'
    raise ModuleNotFoundError(msg) from e

Does this all make sense?

[GenevieveBuckley]

I think this PR is superseded by #44. We can probably safely close this now.

Thank you for all your early work pushing this forward @Lencripe, it is much appreciated

[jni]

Indeed, and thanks @GenevieveBuckley for the reminder here. 😅