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 automatic grid detection to multi #911

Merged
merged 45 commits into from
Jul 15, 2022
Merged

Added automatic grid detection to multi #911

merged 45 commits into from
Jul 15, 2022

Conversation

jdavidpeery
Copy link
Contributor

@jdavidpeery jdavidpeery commented Jun 14, 2022

Can detect a grid from a binary mask without needing to specify top left coordinate, spacing, or radius
Added automatic radius detection if a binary mask is passed.
Added a way to calculate multiple rois from just an image and a mask, but won't have a grid layout.
Refactored to break multi up into several helper functions

Describe your changes
A clear and concise description of what changes are made by this pull request.
What was the previous functionality (if relevant) and what can we do now with
these changes.

Type of update

  • New feature or feature enhancement
  • Work in progress

Associated issues
#906

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

Can detect a grid from a binary mask without needing to specify top left coordinate, spacing, or radius
Added automatic radius detection if a binary mask is passed.
Added a way to calculate multiple rois from just an image and a mask, but won't have a grid layout.
Refactored to break multi up into several helper functions
@jdavidpeery jdavidpeery added enhancement Enhancements to existing features work in progress Mark work in progress update Updates an existing feature/method labels Jun 14, 2022
jdavidpeery and others added 4 commits June 17, 2022 14:46
This is all added to the multi function. We want to split multi up into two functions, but I figure I'd commit this version just in case we want to reference back to it before I split it up into two functions
Also: changes the radius shrinking algorithm to not require a mask.
Also removed some redundancies and unnecessary code. Now img is an optional parameter for auto_grid, i.e. can be done with only a binary mask.
…grid

removed bin_mask from multi- forces the user to specify a radius if only specifying a list of coord, radius is still optional for grid layout

removed shapes parameter - removed rectangular grid functionality, will move to a separate UAV function

Also added doc strings to multi and auto_grid and fixed bugs
added a freaking ,0 that finally fixed it
@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #911 (a429641) into 4.x (5f249a3) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               4.x      #911   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          160       160           
  Lines         6714      6799   +85     
=========================================
+ Hits          6714      6799   +85     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
plantcv/plantcv/__init__.py 100.00% <ø> (ø)
plantcv/plantcv/classes.py 100.00% <100.00%> (ø)
plantcv/plantcv/roi/__init__.py 100.00% <100.00%> (ø)
plantcv/plantcv/roi/roi_methods.py 100.00% <100.00%> (ø)
plantcv/__init__.py 100.00% <0.00%> (ø)

@jdavidpeery jdavidpeery changed the base branch from 4.x to master July 6, 2022 16:24
@jdavidpeery jdavidpeery changed the base branch from master to 4.x July 6, 2022 16:29
Copy link
Member

@nfahlgren nfahlgren left a comment

Choose a reason for hiding this comment

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

Attached code suggestions are mostly code formatting (linting) changes based on Python PEP8 guidelines.

The objects.md docs page should also document the class method append, and the code example could include an example of how to load saved objects.

The roi_multi.md docs page needs to be updated due to the change in the number of return items.

Minor thing, but I would recommend changing the filename test_Objects.py to test_objects.py based on our file naming convention (no uppercases).

plantcv/plantcv/classes.py Outdated Show resolved Hide resolved
plantcv/plantcv/classes.py Outdated Show resolved Hide resolved
plantcv/plantcv/classes.py Outdated Show resolved Hide resolved
plantcv/plantcv/classes.py Outdated Show resolved Hide resolved
plantcv/plantcv/classes.py Show resolved Hide resolved
tests/plantcv/test_Objects.py Outdated Show resolved Hide resolved
tests/plantcv/test_Objects.py Outdated Show resolved Hide resolved
tests/plantcv/test_segment_image_series.py Outdated Show resolved Hide resolved
docs/objects.md Outdated Show resolved Hide resolved
docs/objects.md Outdated Show resolved Hide resolved
jdavidpeery and others added 2 commits July 15, 2022 14:16
Co-authored-by: Noah Fahlgren <noahfahlgren@gmail.com>
@jdavidpeery
Copy link
Contributor Author

jdavidpeery commented Jul 15, 2022

That should address the things you brought up in the review @nfahlgren

docs/roi_multi.md Outdated Show resolved Hide resolved
docs/roi_auto_grid.md Outdated Show resolved Hide resolved
docs/roi_auto_grid.md Outdated Show resolved Hide resolved
docs/roi_auto_grid.md Outdated Show resolved Hide resolved
docs/roi_multi.md Outdated Show resolved Hide resolved
docs/roi_auto_grid.md Outdated Show resolved Hide resolved
return roi_objects, overlap_img, all_roi_img


def auto_grid(bin_mask, nrows, ncols, radius=None, img=None):
Copy link
Member

Choose a reason for hiding this comment

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

I suggest we refactor the input keyword bin_mask. We use two standardized keywords elsewhere, bin_img or mask. Admittedly they pretty much mean the same thing (maybe there's sort of a connotation that mask is a clean binary image vs bin_img is not necessarily filtered yet)

docs/roi_auto_grid.md Outdated Show resolved Hide resolved
docs/roi_auto_grid.md Outdated Show resolved Hide resolved
tests/plantcv/roi/test_roi.py Outdated Show resolved Hide resolved
tests/plantcv/roi/test_roi.py Outdated Show resolved Hide resolved
tests/plantcv/roi/test_roi.py Outdated Show resolved Hide resolved
plantcv/plantcv/roi/roi_methods.py Outdated Show resolved Hide resolved
plantcv/plantcv/roi/roi_methods.py Outdated Show resolved Hide resolved
plantcv/plantcv/roi/roi_methods.py Outdated Show resolved Hide resolved
tests/plantcv/roi/test_roi.py Outdated Show resolved Hide resolved
plantcv/plantcv/roi/roi_methods.py Outdated Show resolved Hide resolved
jdavidpeery and others added 2 commits July 15, 2022 15:56
@nfahlgren
Copy link
Member

This is great @jdavidpeery! It's easy to use and the coding for it all was really well done

@nfahlgren nfahlgren merged commit 4e1bf48 into 4.x Jul 15, 2022
@nfahlgren nfahlgren deleted the roi.multi_rerwite branch July 15, 2022 21:21
@jdavidpeery jdavidpeery mentioned this pull request Jul 20, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancements to existing features ready to review update Updates an existing feature/method
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants