Skip to content

Handle missing FSMap kwargs in FSStore #814

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
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion zarr/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,12 @@ class FSStore(MutableMapping):
as a missing key
dimension_separator : {'.', '/'}, optional
Separator placed between the dimensions of a chunk.
check: bool, optional
Whether to attempt to read from the location before instantiation, to
check that the mapping does exist. Default value is False.
create: bool, optional
Whether to make the directory corresponding to the root before
instantiating. Default value is False.
storage_options : passed to the fsspec implementation
"""

Expand All @@ -1072,10 +1078,12 @@ def __init__(self, url, normalize_keys=False, key_separator=None,
mode='w',
exceptions=(KeyError, PermissionError, IOError),
dimension_separator=None,
check=False,
create=False,
Comment on lines +1081 to +1082
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help me understand why these can't be passed through via **storage_options. I read #775, but here it look like any extra keywords will in the constructor will be passed right to fsspec.get_mapper.

Copy link
Contributor Author

@tasansal tasansal Aug 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rabernat

Do I need to add this change to the release notes as a bug fix, or does someone else handle that before release?

Copy link
Contributor Author

@tasansal tasansal Aug 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe they changed the behavior, let me try it one more time.

Looking at the get_mapper code again,

All **storage_options get passed to url_to_fs function: (link)

The check and create are actually in the FSMap call (link)

**storage_options):
import fsspec
self.normalize_keys = normalize_keys
self.map = fsspec.get_mapper(url, **storage_options)
self.map = fsspec.get_mapper(url, check, create, **storage_options)
self.fs = self.map.fs # for direct operations
self.path = self.fs._strip_protocol(url)
self.mode = mode
Expand Down