Make sure fs exceptions are raised if not Missing

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604

[martindurant]

(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:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[martindurant]

Will pass when fsspec/filesystem_spec#1120 makes it to release

[joshmoore]

Re-launched to see the state of the tests.

[joshmoore]

@martindurant: still red.

[martindurant]

I haven't released fsspec since. Due this week.

[itcarroll]

Suggested change

	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

[itcarroll]

Suggested change

	assertg.store.getitems(["data/1"]) == {} # not found
	withpytest.raises(Exception):
	# None is bad data, as opposed to missing
	g.store.getitems(["data/0", "data/1"])
	assertg.store.getitems(["data/1"], contexts={}) == {} # not found
	withpytest.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

[itcarroll]

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 zarr/test/test_storage.py:TestN5FSStore.test_exceptions but it only fails if pytest has also run TestFSStore. Maybe something involving sharing/conflict with the memory file store?

[jhamman]

This has unfortunately gone stale. Seems like it was quite close. Will it be revived or should we close this?

[itcarroll]

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)!

[jhamman]

@itcarroll - no need for us to close this first. Please go ahead and open a PR of your own with the remaining fixes.

[joshmoore]

Closing this in favor of #1604