Skip to content

Mcc detectors - #1999

Merged
maliagehan merged 16 commits into
v5.0from
mcc-detectors
Oct 17, 2026
Merged

maliagehan merged 16 commits into
v5.0from
mcc-detectors

Conversation

@joshqsumner

@joshqsumner joshqsumner commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Describe your changes
Adds a function to use cv2.mcc detectors in color card detection for times where there are multiple color-cards/similar objects or our other tools are unfamiliar to more cv2 inclined users.

Type of update
This is a new feature.

Associated issues
None

Additional context
This does not support some of the quality of life stuff that pcv.transform.detect_color_card does, namely the size scaling, but I don't see a way around that with how cv2.mcc returns information on the chips.
This also apparently requires opencv-contrib instead of just opencv since only that version has cv2.mcc.

For the reviewer
See this page for instructions on how to review the pull request.

  • PR functionality reviewed in a Jupyter Notebook
  • All tests pass
  • Test coverage remains 100%
  • Documentation tested
  • New documentation pages added to plantcv/mkdocs.yml
  • Changes to function input/output signatures added to updating.md
  • Code reviewed
  • PR approved

@joshqsumner joshqsumner added this to the PlantCV v5.0 milestone Aug 24, 2026
@joshqsumner joshqsumner added the new feature New feature ideas and solutions label Aug 24, 2026
@deepsource-io

deepsource-io Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 06b1625...4283fa6 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Coverage  

Code Review Summary

Analyzer Status Updated (UTC) Details
Python Oct 17, 2026 5:10p.m. Review ↗
Code coverage Oct 17, 2026 5:10p.m. Review ↗

Code Coverage Summary

Language Line Coverage (New Code) Line Coverage (Overall)
Aggregate
100%
[✓ above threshold]
100%
Python
100%
[✓ above threshold]
100%

➟ Additional coverage metrics may have been reported. See full coverage report ↗


Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@maliagehan maliagehan self-assigned this Sep 27, 2026
maliagehan and others added 2 commits September 27, 2026 14:36
added output examples from the function into documentation

@maliagehan maliagehan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests pass, no coverage change, tested in a jupyter notebook (successful), which was added to google drive, added some documentation images since I generated them with testing. I think Noah wanted to have a larger discussion about addtional dependency and how we incorporate CV2 detectors, so won't merge for now.

@nfahlgren nfahlgren added developer check PR/Issue has been reviewed and needs developer input and removed developer check PR/Issue has been reviewed and needs developer input labels Sep 27, 2026

@maliagehan maliagehan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Like the changes to documentation, we discussed a longer/larger change to color correction that might incorporate mcc more seamlessly into color correction workflow, but for now this function works and it's clear when it should be used. Tests pass, documentation looks good, analysis works (notebook is in google drive).

@maliagehan
maliagehan merged commit b1d8e00 into v5.0 Oct 17, 2026
5 checks passed
@maliagehan
maliagehan deleted the mcc-detectors branch October 17, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discussion new feature New feature ideas and solutions ready to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants