Added ArrayNotFoundError #3367
Added ArrayNotFoundError #3367
Conversation
github-actions
bot
added
the
needs release notes
github-actions
bot
removed
the
needs release notes
|
thank you, this looks good. we should make sure we cover all the places in the codebase where we try to read an array. maybe it's just the 1 place you found, but we should double-check. |
dstansby
left a comment
dstansby
left a comment
There was a problem hiding this comment.
Two inline comments. In addition, did you thoroughly check that there's only a single place where an ArrayNotFoundError should be raised?
tests/test_api.py
Outdated
|
|
||
| # path not found | ||
| with pytest.raises(FileNotFoundError): | ||
| with pytest.raises(ArrayNotFoundError): |
There was a problem hiding this comment.
This is currently failing the tests, because it actually raises a GroupNotFoundError now. I think the sensible thing to do here would be to create another new error, NodeNotFoundError, that is raised when a non-specific function is used (e.g., just open) to try and open either an array or a group.
|
it might be helpful to check out #3012, which unfortunately was too bloated to merge |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3367 +/- ##
==========================================
- Coverage 94.66% 94.62% -0.05%
==========================================
Files 79 79
Lines 9444 9463 +19
==========================================
+ Hits 8940 8954 +14
- Misses 504 509 +5
🚀 New features to boost your workflow:
|
tests/test_api.py
Outdated
|
|
||
| # path not found | ||
| with pytest.raises(FileNotFoundError): | ||
| with pytest.raises((NodeNotFoundError, GroupNotFoundError)): |
There was a problem hiding this comment.
how is this raising two errors?
There was a problem hiding this comment.
Yeah fair point, I was getting errors in my tests with only 1 error raised but now I've changed it so GroupNotFoundError and ArrayNotFoundError inherit from NodeNotFoundError
| def __init__(self, *args: Any) -> None: | ||
| if len(args) == 1: | ||
| # Pre-formatted message | ||
| super(BaseZarrError, self).__init__(args[0]) | ||
| else: | ||
| # Store and path arguments - format them | ||
| _msg = "No group found in store {!r} at path {!r}" | ||
| super(BaseZarrError, self).__init__(_msg.format(*args)) |
There was a problem hiding this comment.
in a separate PR we should remove the pre-formatting of exception messages, and make every exception just take a single string
3 participants
Fixes #2961
TODO:
docs/user-guide/*.rstchanges/