Refactor ImageIOLoadException #59
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #59 +/- ##
==========================================
+ Coverage 74.55% 84.35% +9.79%
==========================================
Files 25 35 +10
Lines 849 1355 +506
==========================================
+ Hits 633 1143 +510
+ Misses 216 212 -4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Awesome - thanks @K-Meech
Made some tiny suggestions - tests are looking particularly nice!
| @@ -197,6 +198,9 @@ def load_img_stack( | |||
| logging.debug(f"Loading: {stack_path}") | |||
| stack = tifffile.imread(stack_path) | |||
|
|
|||
| if stack.ndim != 3: | |||
| raise ImageIOLoadException(error_type="2D tiff") | |||
There was a problem hiding this comment.
Could we add the supported data types to the docstring, and specify that 2D tiffs, and paths to folders containing a single tiff or differently shaped tiffs are not supported?
[Edit] Or (better?) specify this by linking to the ImageIOLoadException docs?
There was a problem hiding this comment.
@alessandrofelder I've linked to the ImageIOLoadException docs for the load_any function. For the rest, as only some of the errors in ImageIOLoadException are relevant, I've updated the docstrings directly to make this clearer. Could you take a quick look, and merge if you're happy with the new docstrings?
Description
What is this PR
Why is this PR needed?
brainregcurrently has aLoadFileExceptionclass that should be refactored into brainglobe-utils, and merged with the existingImageIOLoadExceptionWhat does this PR do?
This PR moves the functionality of
LoadFileExceptionintoImageIOLoadException, making sure the relevant load functions catch errors like: loading a single tiff from a directory, loading 2D tiffs etc.References
For brainglobe/brainreg#180
Corresponding PR on
brainregbrainglobe/brainreg#186How has this PR been tested?
Tests were updated, and all pass locally.
Is this a breaking change?
No
Does this PR require an update to the documentation?
No
Checklist: