-
Notifications
You must be signed in to change notification settings - Fork 32
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
[BUG] Handle exception when the image is too small after resizing for minimum patch sizes #136
Conversation
…d within a script with arguments (for tests), provide a return value code for success, and test for working case
@jcohenadad this PR resolves an old issue you raised (#34), so I assigned you as the reviewer a while back. Do you have time to check it? If not I can assign @MelanieLu to do so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, it does the trick. There is just a minor change of behaviour when the user does not input "-s" flag:
on master (2b2a15b):
axondeepseg -t TEM -i Does_Kelm_072214_021.tif -v 2
AxonDeepSeg v.2.1
Exception: ERROR: No pixel size is provided, and there is no pixel_size_in_micrometer.txt file in image folder. Please provide a pixel size (using argument -s), or add a pixel_size_in_micrometer.txt file containing the pixel size value.
this PR (1086204):
axondeepseg -t TEM -i Does_Kelm_072214_021.tif -v 2
AxonDeepSeg v.2.2dev
EXCEPTION: The size of one of the images (2304x1888) is too small for the provided pixel size (0.0).
The image size must be at least 512x512 after resampling to a resolution of 0.01 to create standard sized patches.
One of the dimensions of the image has a size of 0 after resampling to that resolution.
Image file location: Does_Kelm_072214_021.tif
@jcohenadad Ah ha, nice catch. This happens because the default CLI pixel size value was set to 0 when not value was given: It throws my error because evidently a pixel size of 0 is smaller than the minimum size, and the other error you mention is only thrown much further in the pipeline and in another file: instead of at the beginning like mine. They used the exact value of 0 as a an indicator of no given input. I'll find a workaround for this – all exception handling for input parameters should be done in segment.py anyways, which is the file that interfaces with the command line. |
…uropoly/axondeepseg into resolution_exception_handling
…re only a folder is given to segment. Add additional integration tests
@jcohenadad I think both cases are resolved now. I also implemented the exception handling for when a folder is provided instead of a direct link to the image – I had missed that/those case(s) too. All the cases (successes & exceptions) are now also covered by the testing suite, but check by yourself on the command line too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested without -s
and it now produces the expected behaviour. Awesome job @mathieuboudreau 👍
however i ventured to uncharted territories (as always), and created this bug (#138), which we can addrss in another PR.
Resolves #34 , and also contributes to #29
The image dimensions are checked, and a descriptive message is printed if certain conditions are not met.
Two tests were added to make sure that an exit code 0 is given for a successful run, and an error code 2 for bad image size/resolution values (code 1 will be used later for invalid/unknown CLI inputs/options).
Example using the file from the issue using too small of a resolution:
As mentioned in the issue, the processing work as expected for the actual resolution of that file: