-
Notifications
You must be signed in to change notification settings - Fork 271
Added fatal_error for incorrect block_size value in detect_color_card #1457
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
Added fatal_error for incorrect block_size value in detect_color_card #1457
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1457 +/- ##
=======================================
Coverage 99.98% 99.98%
=======================================
Files 159 159
Lines 7048 7050 +2
=======================================
+ Hits 7047 7049 +2
Misses 1 1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Thanks for opening this Pull Request @tj-schultz. I think the informative fatal error is a good way to handle this, as more information in the doc page for a totally optional parameter could be a distraction. For some reason, DeepSource is complaining about the level of complexity being too high for this function now. Not sure how they are calculating this as this function looks far less complex than others in this package. Also not sure it's something worth addressing, since I'd consider the functionality well documented and easy enough to edit as is. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1457 +/- ##
==========================================
- Coverage 99.98% 99.98% -0.01%
==========================================
Files 161 159 -2
Lines 7066 7050 -16
==========================================
- Hits 7065 7049 -16
Misses 1 1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Here's the code health analysis summary for commits Analysis Summary
|
It appears that the Test Coverage artifact cannot be reported from DeepSource as the introduction of the failing metric is preventing the merging of this PR. There also appear to be other cyclomatic complexity threshold risks in the existing plantcv source for other functions. json2csv, segment_insertion_angle, etc. One solution for this PR and perhaps to address these other instances would be to manually set the threshold for the project. From DeepSource:
This would be a decision for the entire project and is larger than the scope of this PR, but could make it easier to amend existing components in the future. |
This reverts commit 628c9dd.
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.
Thanks @tj-schultz!
Describe your changes
Added logic to check for incorrect block_size values to pass to cv2.adaptiveThreshold inside detect_color_card. Throws a fatal_error if not odd or greater than 1.
Type of update
Associated issues
#1426
Additional context
Documentation was left unchanged for simplicity. Could still be described in detect_color_card docs.
For the reviewer
See this page for instructions on how to review the pull request.
plantcv/mkdocs.yml
updating.md