-
-
Notifications
You must be signed in to change notification settings - Fork 282
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
Make sure fs exceptions are raised if not Missing #1237
Conversation
Will pass when fsspec/filesystem_spec#1120 makes it to release |
Re-launched to see the state of the tests. |
@martindurant: still red. |
I haven't released fsspec since. Due this week. |
arr = g.create_dataset("data", data=[1, 2, 3, 4], | ||
dtype="i4", compression=None, chunks=[2]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arr = g.create_dataset("data", data=[1, 2, 3, 4], | |
dtype="i4", compression=None, chunks=[2]) | |
arr = g.create_dataset("data", data=[1, 2, 3, 4], dtype="i4", compression=None, chunks=[2]) |
black/ruff line length is 100, this will fix one red x
assert g.store.getitems(["data/1"]) == {} # not found | ||
with pytest.raises(Exception): | ||
# None is bad data, as opposed to missing | ||
g.store.getitems(["data/0", "data/1"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert g.store.getitems(["data/1"]) == {} # not found | |
with pytest.raises(Exception): | |
# None is bad data, as opposed to missing | |
g.store.getitems(["data/0", "data/1"]) | |
assert g.store.getitems(["data/1"], contexts={}) == {} # not found | |
with pytest.raises(Exception): | |
# None is bad data, as opposed to missing | |
g.store.getitems(["data/0", "data/1"], contexts={}) |
missing required contexts
kwarg is causing one (of two) test failures
Red because of (at least) two test failures and one pre-commit.ci formatting check. I've added comments to address two of those. One test failure is confusing. It's |
This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this? |
If you do close, I have a fork with the commits from @martindurant and fixes from my review. If I can resolve the 3rd failure, I will create a new PR (although I hope OP beats me to it)! |
@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes. |
Closing this in favor of #1604 |
(for v2 for now)
When FSStore gets exceptions, these correctly become the fill value when they are translated to KeyError. However, some exceptions do not equate to missing keys but to other permanent problems such as PermissionError. This change makes sure they are correctly raised to the caller even within getitems (getitem already was correct).
TODO: