Skip to content

Update fsspec minimal version? #735

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

Closed
joshmoore opened this issue May 4, 2021 · 9 comments · Fixed by #2534
Closed

Update fsspec minimal version? #735

joshmoore opened this issue May 4, 2021 · 9 comments · Fixed by #2534

Comments

@joshmoore
Copy link
Member

I've just gotten this user report:

    cdatas = self.chunk_store.getitems(ckeys, on_error="omit")
  File "c:\users\c\napari\lib\site-packages\zarr\storage.py", line 1085, in getitems
    return self.map.getitems(keys, on_error="omit")
AttributeError: 'FSMap' object has no attribute 'getitems'
(napari) PS C:\Users\callan> pip show zarr
Name: zarr
Version: 2.8.1
Summary: An implementation of chunked, compressed, N-dimensional arrays for Python.
Home-page: https://github.com/zarr-developers/zarr-python
Author: None
Author-email: None
License: MIT
Location: c:\users\callan\napari\lib\site-packages
Requires: numcodecs, fasteners, asciitree, numpy
Required-by: ome-zarr, omero-napari

Is it possible that we need to (A) update the fsspec pin and (B) perhaps include it as a requirement?

requirements_dev_optional.txt:fsspec==0.8.4; python_version > '3.6'

cc: @martindurant

@martindurant
Copy link
Member

fsspec is a pretty small requirement, since it has no deps of its own, but many zarr users won't need it.
For an optional requirement, yes I would update. 0.9.0 includes getitems, but why not just keep up with latest.

@joshmoore joshmoore changed the title Update fsspec pin? Update fsspec minimal version? May 4, 2021
@joshmoore
Copy link
Member Author

but why not just keep up with latest.

Sorry, I didn't mean pin, but rather set a minimal version. 👍 for going >= 0.9.0. If there are other votes for dropping optional, speak up.

@joshmoore
Copy link
Member Author

Oh dear. This may end up being something of a can-of-worms. All of the versions in requirements_dev_optional.txt are pinned, and I imagine they haven't been updated since appveyor was dropped in gh-706. The activation of dependabot (gh-734) should kick off the process again.

@MatthewBM
Copy link

MatthewBM commented May 12, 2021

I'm having stability issues with both fsspec<=0.8.0 and fsspec>=0.9.0. Only fsspec == 0.8.7 is working for me currently using Zarr and S3

fsspec<=0.8.0 cannot import maybe_sync:
https://www.gitmemory.com/issue/intake/filesystem_spec/597/829392517

 from fsspec.asyn import AsyncFileSystem, sync, sync_wrapper, maybe_sync
ImportError: cannot import name 'maybe_sync'

fsspec>=0.9.0 cannot use on_error because fsspec's map operator no longer takes on_error arg
awslabs/amazon-asdi#21

~/anaconda3/envs/satpy/lib/python3.7/site-packages/zarr/core.py in _chunk_getitems()
   1689 
   1690         ckeys = [self._chunk_key(ch) for ch in lchunk_coords]
-> 1691         cdatas = self.chunk_store.getitems(ckeys, on_error="omit")
   1692         for ckey, chunk_select, out_select in zip(ckeys, lchunk_selection, lout_selection):
   1693             if ckey in cdatas:

TypeError: getitems() got an unexpected keyword argument 'on_error'

@martindurant
Copy link
Member

I don't think there would be any downside to depending on very recent fsspec

@joshmoore
Copy link
Member Author

Ok. I've activated dependabot (#734) which will have us testing with newer versions, but is there any way to give guidance to users on these dependencies, @martindurant?

@martindurant
Copy link
Member

It's only fairly recently that this functionality got into zarr anyway, with previous docs describing how to make your own mapper from fsspec itself - so I'm not to worried about that.

@joshmoore
Copy link
Member Author

@MatthewBM are things working again?

@joshmoore
Copy link
Member Author

I'm leaning towards closing this based on #734 since we should be able to detect when a new version of fsspec* breaks things.

But that won't necessarily help users to know when they need to update. Allowing pip install zarr[fsspec] with the right minimal version might be one way to distribute this knowledge. Any opinions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants