Skip to content
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

Add checks for empty arrays post-segmentation #4199

Merged
merged 6 commits into from Aug 24, 2023

Conversation

joshuacwnewton
Copy link
Member

Description

This PR provides more intuitive feedback when sct_deepseg_sc produces an empty array. This avoids a more unintuitive error later on in the postprocessing function.

Linked issues

Fixes #4197.

Copy link
Member

@mguaypaq mguaypaq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These error messages look much better, thanks.

Since they're uncaught exceptions, they'll appear to the user with a possibly-confusing traceback. Could you add a try...except block in the CLI layer to instead display the associated error message using printv(..., 'error')? For example:

except ValueError as e:
printv(f'Vertebral detection failed: {e}', 1, 'error')
sys.exit(1)

Copy link
Member

@mguaypaq mguaypaq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine, thanks! And yes, if we're going to have some general-purpose custom exceptions, types.py seems like a fine place for them. I think, as time goes on, we'll see whether or not we like this error-handling pattern.

@mguaypaq mguaypaq merged commit 33c924e into master Aug 24, 2023
24 checks passed
@mguaypaq mguaypaq deleted the jn/4197-sct_deepseg_sc-min-error branch August 24, 2023 15:50
@joshuacwnewton joshuacwnewton added documentation category: readthedocs, sourceforge, or SCT courses sct_deepseg context: Global entry point for all deep learning segmentation methods enhancement category: improves performance/results of an existing feature and removed documentation category: readthedocs, sourceforge, or SCT courses labels Oct 30, 2023
@joshuacwnewton joshuacwnewton added this to the 6.1 milestone Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement category: improves performance/results of an existing feature sct_deepseg context: Global entry point for all deep learning segmentation methods
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sct_deepseg_sc fails with unintuitive error when no segmentation is detected
2 participants