ENH: Exception fixes and bounds checking #34

tbirdso · 2023-07-22T17:08:04Z

Changes:

Replace assert messages with itkAssertOrThrowMacro to emit itk::ExceptionObjects on failure, which are handled in ITKOMEZarrNGFF unit tests and provide line references with error output in ITK Python.
Clarifies file read errors. Previous messages indicated JSON failure rather than missing file
Previous behavior:

RuntimeError: [json.exception.out_of_range.403] key 'multiscales' not found

Updated behavior:

RuntimeError: D:\repos\ITKIOOMEZarrNGFF\src\itkOMEZarrNGFFImageIO.cxx:374:
ITK ERROR: Failed to read from C:/bad/path/.zgroup

Resolves handling where requested index is equal to the size of available datasets and thus out of bounds.
Previous behavior:

RuntimeError: D:\a\im\src\itkOMEZarrNGFFImageIO.cxx:395:
ITK ERROR: OMEZarrNGFFImageIO(000001A91BDBD060): OME-NGFF v0.4 requires `coordinateTransformations` for each resolution level.

Updated behavior:

ITK ERROR: OMEZarrNGFFImageIO(0000015D06528500): Requested DatasetIndex of 4 is greater than number of datasets (4) which exist in OME-NGFF store

Replaces `assert` statements with `itkAssertOrThrowMacro` for better ITK exception handling. Addresses issue where opaque JSON parse errors were thrown in testing that could not be handled as ITK exceptions.

Updates error message for metadata read failure where required OME-Zarr NGFF metadata files `.zattrs` and `.zgroup` are not present in the specified directory. Under previous behavior the JSON read status was not examined, so it appeared that metadata was read in successfully with missing keys.

dzenanz

Mostly looks good.

src/itkOMEZarrNGFFImageIO.cxx

Updates dataset index bounds check to reflect zero indexed dataset list. Under previous logic an index equivalent to the number of datasets would succeed with apparent missing JSON keys. Updates associated error message for clarity.

Resolve GitHub Actions error: ``` CMake Warning (dev) at /home/runner/work/ITKIOOMEZarrNGFF/ITKIOOMEZarrNGFF/dashboard.cmake:18 (set): Syntax error in cmake code at /home/runner/work/ITKIOOMEZarrNGFF/ITKIOOMEZarrNGFF/dashboard.cmake:28 when parsing string _deps[/\].+-src[/\] Invalid escape sequence \] ```

tbirdso added 2 commits July 22, 2023 11:46

COMP: Use ITK asserts for exception handling

d6755fc

Replaces `assert` statements with `itkAssertOrThrowMacro` for better ITK exception handling. Addresses issue where opaque JSON parse errors were thrown in testing that could not be handled as ITK exceptions.

tbirdso requested review from dzenanz and thewtex July 22, 2023 17:08

tbirdso mentioned this pull request Jul 22, 2023

Empty multiscales/multiscaleTransformations metadata field thewtex/ngff-zarr#49

Closed

tbirdso force-pushed the exception-fixes branch from 0e1c8a5 to 1ffc8aa Compare July 22, 2023 22:58

dzenanz reviewed Jul 23, 2023

View reviewed changes

src/itkOMEZarrNGFFImageIO.cxx Outdated Show resolved Hide resolved

BUG: Raise exception on out of range dataset index

e7ff3d5

Updates dataset index bounds check to reflect zero indexed dataset list. Under previous logic an index equivalent to the number of datasets would succeed with apparent missing JSON keys. Updates associated error message for clarity.

tbirdso force-pushed the exception-fixes branch from 1ffc8aa to e7ff3d5 Compare July 24, 2023 13:01

thewtex approved these changes Jul 24, 2023

View reviewed changes

tbirdso merged commit 222fa8e into InsightSoftwareConsortium:master Jul 24, 2023
18 checks passed

tbirdso deleted the exception-fixes branch July 25, 2023 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Exception fixes and bounds checking #34

ENH: Exception fixes and bounds checking #34

tbirdso commented Jul 22, 2023

dzenanz left a comment

ENH: Exception fixes and bounds checking #34

ENH: Exception fixes and bounds checking #34

Conversation

tbirdso commented Jul 22, 2023

dzenanz left a comment

Choose a reason for hiding this comment