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

Implement Younger Monocot Pipeline #64

Merged
merged 13 commits into from
Sep 15, 2023
Merged

Conversation

eberrigan
Copy link
Collaborator

@eberrigan eberrigan commented Sep 13, 2023

  • Added YoungerMonocotPipeline as subclass of Pipeline
  • Test YoungerMonocotPipeline

Summary by CodeRabbit

  • New Feature: Introduced YoungerMonocotPipeline class for trait computation of younger monocot plants, including root lengths, angles, intersections, and network length.
  • Refactor: Updated get_grav_index function in sleap_roots/lengths.py to handle both scalar and array inputs uniformly, improving code flexibility and robustness.
  • Test: Added comprehensive test cases for get_grav_index function covering various scenarios such as scalar inputs, array inputs, shape mismatch, NaN values, and zero lengths.
  • Test: Implemented a new test function test_younger_monocot_pipeline to validate the functionality of the newly introduced YoungerMonocotPipeline class.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 13, 2023

Walkthrough

The changes primarily focus on enhancing the functionality of the get_grav_index function and introducing a new class YoungerMonocotPipeline. The get_grav_index function now handles scalar and array inputs uniformly, while the YoungerMonocotPipeline class defines a trait computation pipeline for younger monocot plants. Test cases have been updated and expanded to cover these changes.

Changes

File Summary
sleap_roots/lengths.py Updated get_grav_index function to handle scalar and array inputs uniformly. Added error handling for shape mismatch between input arrays.
sleap_roots/trait_pipelines.py Introduced a new class YoungerMonocotPipeline that extends the Pipeline class. It defines a trait computation pipeline for younger monocot plants.
tests/test_lengths.py Updated test cases for the get_grav_index function to cover different scenarios such as scalar inputs, array inputs, shape mismatch, NaN values, zero lengths, and invalid scalar values.
tests/test_trait_pipelines.py Added a new test function test_younger_monocot_pipeline that tests the YoungerMonocotPipeline class.

🐇💻

Code refined, tests designed,

In the land of roots and vines.

Scalar or array, come what may,

Our code is ready to play!

With pipelines new, and grav index too,

Celebrate we do, this coding brew! 🎉


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 4

Commits Files that changed from the base of the PR and between 27bbe85 and 0feee0b commits.
Files selected for processing (4)
  • sleap_roots/lengths.py (2 hunks)
  • sleap_roots/trait_pipelines.py (1 hunks)
  • tests/test_lengths.py (2 hunks)
  • tests/test_trait_pipelines.py (2 hunks)
Additional comments (Suppressed): 11
tests/test_trait_pipelines.py (1)
  • 1-5: The import of YoungerMonocotPipeline and find_all_series is new. Ensure that these classes/functions are correctly implemented and tested.
tests/test_lengths.py (6)
  • 165-166: This test case for scalar inputs in the get_grav_index function is well written and covers an important scenario.

  • 169-174: This test case for array inputs in the get_grav_index function is well written and covers an important scenario.

  • 177-183: This test case correctly checks for a ValueError when there is a shape mismatch between the input arrays to the get_grav_index function.

  • 186-191: This test case correctly checks the handling of NaN values in the get_grav_index function.

  • 194-199: This test case correctly checks the handling of zero lengths in the get_grav_index function.

  • 202-206: These test cases correctly check the handling of invalid scalar values in the get_grav_index function.

sleap_roots/trait_pipelines.py (3)
  • 904-905: The new YoungerMonocotPipeline class is a subclass of the Pipeline class and is designed to compute traits for younger monocot plants. This is a good addition as it allows for more specific trait computation for different types of plants.

  • 920-1343: The define_traits method has been overridden in the YoungerMonocotPipeline class to define the trait computation pipeline specific to younger monocot plants. The method returns a list of TraitDef objects representing the traits to be computed. Each TraitDef object includes a name, a function (fn) that computes the trait, a list of input traits required for computing the trait, flags indicating whether the trait is scalar or should be included in downstream CSV files, additional keyword arguments for the computation function, and a description of the trait. This is a comprehensive set of traits for younger monocot plants and covers various aspects such as root lengths, angles, intersections, network length, etc.

  • 1345-1373: The get_initial_frame_traits method has been updated to return initial traits for a plant frame. It takes a plant Series object and a frame_idx as inputs and returns a dictionary of initial traits with keys: "primary_pts" and "main_pts". The method handles cases where there are no instances for primary and main roots by returning arrays with NaN values. This is a good practice as it ensures that the output shape is consistent regardless of the input.

sleap_roots/lengths.py (1)
  • 1-6: The import statement for get_base_tip_dist has been removed. Ensure that this function is no longer used in the codebase or has been replaced with an equivalent functionality.

tests/test_trait_pipelines.py Outdated Show resolved Hide resolved
tests/test_lengths.py Outdated Show resolved Hide resolved
sleap_roots/trait_pipelines.py Outdated Show resolved Hide resolved
sleap_roots/lengths.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #64 (3a005a2) into main (0e7f824) will increase coverage by 1.24%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
+ Coverage   77.05%   78.30%   +1.24%     
==========================================
  Files          13       13              
  Lines         789      802      +13     
==========================================
+ Hits          608      628      +20     
+ Misses        181      174       -7     
Files Changed Coverage Δ
sleap_roots/trait_pipelines.py 92.90% <90.00%> (-0.44%) ⬇️
sleap_roots/lengths.py 91.11% <100.00%> (+14.18%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Commits Files that changed from the base of the PR and between 0feee0b and a1a5550 commits.
Files selected for processing (4)
  • sleap_roots/lengths.py (2 hunks)
  • sleap_roots/trait_pipelines.py (1 hunks)
  • tests/test_lengths.py (2 hunks)
  • tests/test_trait_pipelines.py (2 hunks)
Files skipped from review due to trivial changes (2)
  • sleap_roots/trait_pipelines.py
  • tests/test_lengths.py
Additional comments (Suppressed): 2
tests/test_trait_pipelines.py (1)
  • 1-5: The import of YoungerMonocotPipeline and find_all_series is new in this hunk. Ensure that these classes/functions are correctly implemented and tested elsewhere in the codebase.
sleap_roots/lengths.py (1)
  • 1-6: The import of get_base_tip_dist from sleap_roots.bases has been removed. Ensure that this function is not used elsewhere in the code or if it's used, it's imported correctly.

tests/test_trait_pipelines.py Outdated Show resolved Hide resolved
sleap_roots/lengths.py Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Commits Files that changed from the base of the PR and between a1a5550 and 28abf45 commits.
Files selected for processing (1)
  • tests/test_trait_pipelines.py (2 hunks)
Additional comments (Suppressed): 1
tests/test_trait_pipelines.py (1)
  • 1-5: The import of YoungerMonocotPipeline and find_all_series is new in this hunk. Ensure that these classes/functions are correctly implemented and tested elsewhere in the codebase.

tests/test_trait_pipelines.py Show resolved Hide resolved
tests/test_trait_pipelines.py Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between 28abf45 and 3a005a2 commits.
Files selected for processing (2)
  • sleap_roots/lengths.py (2 hunks)
  • tests/test_lengths.py (2 hunks)
Files skipped from review due to trivial changes (1)
  • tests/test_lengths.py
Additional comments (Suppressed): 1
sleap_roots/lengths.py (1)
  • 1-6: The import of get_base_tip_dist from sleap_roots.bases has been removed. Ensure that this function is no longer used in the codebase or has been replaced appropriately.

sleap_roots/lengths.py Show resolved Hide resolved
Copy link
Contributor

@talmo talmo left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@eberrigan eberrigan merged commit 86fca8a into main Sep 15, 2023
5 checks passed
@eberrigan eberrigan deleted the elizabeth/young_monocot_pipeline branch September 15, 2023 17:28
eberrigan added a commit that referenced this pull request Mar 14, 2024
* Start `YoungerMonocotPipeline`

* Add `main_grav_indices`

* Add younger monocot data

* Edit `YoungMonocotPipeline`

* Test `YoungerMonocotPipeline`

* Fix trait definitions

* Fix description of `main_grav_indices`

* Test gravitropism index

* Fixed test for `grav_index` for NaN values

* Add tests for `get_grav_index` for expected output shape
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.

2 participants