Remove background averaging #169
Conversation
Codecov Report
@@ Coverage Diff @@
## main #169 +/- ##
==========================================
+ Coverage 11.18% 11.21% +0.02%
==========================================
Files 45 45
Lines 5997 5984 -13
==========================================
Hits 671 671
+ Misses 5326 5313 -13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Preview page for your plugin is ready here: |
|
Do you think a warning is still needed since we are altering the behavior of the pipeline? |
|
I think I removed the warning and decided to completely remove the background averaging. I initially thought I was going to just add a warning, then I changed my mind and decided to remove background averaging. Sorry I didn't do a very good job cleaning up after myself: the warning is still in the commit history and in the name of this branch. |
I wasn't making it clear, but I was suggesting adding the warning back (with different articulation) so that any user who was relying on that bug/feature would be advised of the change we are making. Although the previous behavior was more like a bug, this 'breaking' change still alters the output value of the pipeline if, for example, someone would like to revisit their previous analysis. |
|
Aha! Excellent point. I didn't maintain backwards compatibility. I've reverted the changes, added a warning, fixed the bug, and added reminders for what needs to be removed for 1.0.0. Thanks Ziwen...great catch. |
recOrder/io/utils.py
Outdated
| Warning: earlier versions of recOrder would have averaged over the background ROI. This behavior is | ||
| now considered a bug, and future versions of recOrder will not average over the background. | ||
| """ | ||
| print(warning_msg) |
There was a problem hiding this comment.
Same question as in this comment, as this message may not be visible from the napari window.
There was a problem hiding this comment.
Again...good catch. Changed from print to logging.warning.
When I was testing calibration using an ROI, I noticed that the background correction changed behavior.
I found that when a calibration ROI was used, the background correction averaged over this ROI then applied that background correction uniformly over the whole image. I cannot think of a reason to average over a FOV within a background image, so I removed this functionality.
This functionality severely reduces the effectiveness of the "Measured" background corrections when calibrating within an ROI. It's also likely that this bug was partially responsible for Yuming's poor background corrections.