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 Tests for Untangle.R #128

Merged
merged 21 commits into from
Oct 30, 2024
Merged

Added Tests for Untangle.R #128

merged 21 commits into from
Oct 30, 2024

Conversation

alecbuetow
Copy link
Contributor

I tried to achieve full coverage but I don't think it will be possible for all_couple_rotations_at_k lines 523, 528-534. I believe neither k_cluster_leaves nor km1_cluster_leaves can be NA, since both variables depend on the function cutree() which has the default parameter NA_to_0L = TRUE, setting all NA values to 0 instead.

I also had to change untangle_intercourse to create a working test for untangle_evolution. I think the corrections make the function more logically consistent; as it currently stands, untangle_intercourse takes four dendrograms as its input, then untangles #1 and #4 together, then #2 and #3 together. When it returns your newly untangled dendrograms, they are in the order: 1, 4, 3, 2. The other comparable functions to this one all untangle #1 and #2 together, then #3 and #4 together, returning them in the order: 1, 2, 3, 4. My changes made untangle_intercourse do the same.

Copy link

codecov bot commented Oct 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.77%. Comparing base (f3a27a6) to head (596a3b4).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   39.83%   41.77%   +1.93%     
==========================================
  Files          50       50              
  Lines        4182     4182              
==========================================
+ Hits         1666     1747      +81     
+ Misses       2516     2435      -81     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@talgalili talgalili merged commit 546c3c2 into talgalili:master Oct 30, 2024
17 checks passed
@talgalili
Copy link
Owner

All sounds good to me.
I'm not sure you should have changed the dendlist to list (there are other ways to have fixed it for testing).
But I approved all changes for now.

Thanks for the work!
I'd love to see more diffs :)

@alecbuetow
Copy link
Contributor Author

Amazing, I'm happy to go back and modify the function and tests if you'd like to use a dendlist. Won't be hard to modify it now that everything else is already set

@talgalili
Copy link
Owner

talgalili commented Oct 30, 2024 via email

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