fiji / Colocalisation_Analysis

Fiji's plugin for colocalization analysis
http://imagej.net/Coloc_2
GNU General Public License v3.0
24 stars 18 forks source link

Added warnings in case of negative pixel values #49

Closed haesleinhuepf closed 7 years ago

haesleinhuepf commented 7 years ago

I added two checks preventing a crash and warning the user when using images with negative pixel values to resolve #48 .

Cheers, Robert

chalkie666 commented 7 years ago

@haesleinhuepf I tidied up the warning text comments a bit, and clarified the new one a little. Is it understandable?

The code changes look ok to me... i suppose you tested it works and catches the -ve pixel values issue right and prevents the crash? Right?

Should we have a unit test to check this is working? Or is that overkill?

I'm happy to merge if you are confident its all good.

haesleinhuepf commented 7 years ago

@chalkie666 I just pulled and checked again. I works as expected. The error message appears more informative now. But is it a good idea to suggest setting negative pixel values to zero? I mean, technically you can make the algorithm work, but the results may stay as wrong was when these pixels are ignored, right?

haesleinhuepf commented 7 years ago

Ah and regarding the automated test: I would like to add a test, but appears quite complicated as there is no InputCheck test I could just extend. So maybe we keep a another github issue "Write test for InputCheck class"?

chalkie666 commented 7 years ago

@haesleinhuepf Actually the input checks are tests in of themselves... so probably no point in testing tests? Maybe my logic is wrong here... Depending on the type of microscopy, and if it involved a reconstruction algorithm to generate final image, it might make sense to shift values or clamp negatives to zero.... if we offer two possibilities, at least the user is aware they need to think harder. The warning is the best help we can give.

Thanks for the contribution here! Its a worthwhile problem to catch, and now we do!