Skip to content
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 background image input to visualize.pseudocolor #1680

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rayn-alex
Copy link
Collaborator

Describe your changes
Added an option/input for a RGB image that will be uses as a background in plantcv.visualize.pseudocolor()

Type of update
Is this a:

  • New feature or feature enhancement

Associated issues
Closes #1650

Additional context
Add any other context about the problem here.

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

@rayn-alex rayn-alex requested a review from HaleySchuhl March 7, 2025 11:10
@rayn-alex rayn-alex self-assigned this Mar 7, 2025
Copy link

deepsource-io bot commented Mar 7, 2025

Here's the code health analysis summary for commits 432fcd2..2046fb7. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Python LogoPython✅ SuccessView Check ↗
DeepSource Test coverage LogoTest coverage✅ SuccessView Check ↗

Code Coverage Report

MetricAggregatePython
Branch Coverage100%100%
Composite Coverage100%100%
Line Coverage100%100%
New Branch Coverage100%100%
New Composite Coverage100%100%
New Line Coverage100%, ✅ Above Threshold100%, ✅ Above Threshold

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@rayn-alex
Copy link
Collaborator Author

This is how it looks 🙂
image

@rayn-alex
Copy link
Collaborator Author

Haha, ok, DeepSource is a little picky about the white space in the docstring. I'll fix that. I also forgot to add this to the documentation

@rayn-alex
Copy link
Collaborator Author

@HaleySchuhl I'm missing an example image for the documentation. I already prepared everything in the md file, but I could not find the original image that was used for the visualize.pseudocolor() doc page.

I'm calling "img/documentation_images/pseudocolor/pseudo_on_rgb_image.jpg" as image, but this image does not exist yet.

@HaleySchuhl
Copy link
Contributor

HaleySchuhl commented Mar 7, 2025

edited:
Thanks @rayn-alex 👏 the example image here looks awesome! I found the original images used in that doc page but there's not an RGB version of that Top View plant, not with the same field of view anyways. There's no harm in using a different example for the RGB use-case in these docs, and I think you're example image above is great if you'd be willing to use that.

We also discussed this morning that adding a new parameter bg_image is technically a breaking change, which we are trying to avoid between major version releases. Instead, it might be most intuitive to update the existing background parameter to accept either `"white", "black", "image", OR an array object. Then if the array object is provided, it'll be used for background. Thoughts?

@rayn-alex
Copy link
Collaborator Author

This absolutely makes sense!
I'll make the edits.

@rayn-alex
Copy link
Collaborator Author

@HaleySchuhl
I just started working on this and realized that introducing this in the background variable requires substancial changes in the code.

The whole magic of this part needs to be replaced, because background is not always a string anymore.

Is this fine with you?

# Set the background color or type
    bkgd = {
        "BLACK": {
            "image": np.zeros(np.shape(gray_img1), dtype=np.uint8),
            "cmap": "gray"
            },
        "WHITE": {
            "image": np.zeros(np.shape(gray_img1), dtype=np.uint8) + 255,
            "cmap": "gray_r"
            },
        "IMAGE": {
            "image": bg_image1 if bg_image1 is not None else gray_img1,
            "cmap": None if bg_image1 is not None else "gray"
            }
    }
    bkg_img = bkgd[background.upper()]["image"]
    bkg_cmap = bkgd[background.upper()]["cmap"]

@rayn-alex
Copy link
Collaborator Author

Hey @HaleySchuhl
I just moved the whole background checking block a bit lower, and then things did not look as horrible as I expected.

Just one thing:
Should we check for the input image dimensions in this part of the code? There will be an error if the gray x/y shape does not fit to the background array x/y from matplotlib. The question is if we should turn it into something more meaningful for the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow independent background image in plantcv.visualize.pseudocolor() and others.
2 participants