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

Fix get_network_distribution #67

Merged
merged 2 commits into from
Sep 22, 2023
Merged

Conversation

eberrigan
Copy link
Collaborator

@eberrigan eberrigan commented Sep 22, 2023

  • Add check to get_network_distribution for roots with less than one point
  • Add tests for get_network_distribution

Summary by CodeRabbit

  • Bug Fix: Updated get_network_distribution function in sleap_roots/networklength.py to handle roots with only one point, ensuring accurate network length calculations.
  • Test: Added comprehensive test cases for get_network_distribution in tests/test_networklength.py, covering scenarios including single-point networks, empty arrays, NaN values, and various fraction values. This enhances the robustness of our testing suite.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 22, 2023

Walkthrough

The changes primarily focus on enhancing the robustness of the get_network_distribution function and its corresponding tests. The function now handles edge cases more effectively, ensuring only valid roots are processed. Additionally, new test scenarios have been introduced to cover a wider range of inputs, thereby improving the overall reliability of the code.

Changes

File Summary
.../networklength.py Removed unused import and added checks for valid roots in get_network_distribution.
tests/test_networklength.py Added new test cases for get_network_distribution, covering various scenarios including single-point networks, empty arrays, NaN values, monocots, and different fraction values.

🐇💻

In the land of code where the shadows lie,

A rabbit hopped forth, his gaze held high.

With each line he tweaked, with each test he spun,

He made sure the battle against bugs was won.

Now the code is stronger, the logic more tight,

Celebrate we shall, under the moon's soft light! 🌙🎉


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: 0

Commits Files that changed from the base of the PR and between 7f92031 and bfdb265.
Files selected for processing (2)
  • sleap_roots/networklength.py (2 hunks)
  • tests/test_networklength.py (2 hunks)
Files skipped from review due to trivial changes (1)
  • tests/test_networklength.py
Additional comments (Suppressed): 2
sleap_roots/networklength.py (2)
  • 5-5: The import statement for get_root_lengths has been removed. Please verify that this function is no longer used in the codebase and that removing it doesn't affect any other parts of the system.

  • 201-205: The new check ensures that only roots with more than one point are considered for calculations. This is a good practice as it prevents potential errors or inaccuracies when creating LineString objects from single-point roots.

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Merging #67 (bfdb265) into main (7f92031) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #67      +/-   ##
==========================================
+ Coverage   78.30%   78.45%   +0.15%     
==========================================
  Files          13       13              
  Lines         802      803       +1     
==========================================
+ Hits          628      630       +2     
+ Misses        174      173       -1     
Files Changed Coverage Δ
sleap_roots/networklength.py 82.92% <100.00%> (+1.44%) ⬆️

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

Copy link
Collaborator

@linwang9926 linwang9926 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@eberrigan
Copy link
Collaborator Author

This PR fixes #66.

@eberrigan eberrigan merged commit fd41164 into main Sep 22, 2023
5 checks passed
@eberrigan eberrigan deleted the elizabeth/fix_network_length branch September 22, 2023 21:28
eberrigan added a commit that referenced this pull request Mar 14, 2024
* Added check to `get_network_distribution` for root length > 1

* Added tests for `get_network_distribution`
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