talgalili / dendextend

Extending R's Dendrogram Functionality
153 stars 29 forks source link

Test Cases Added #130

Closed alecbuetow closed 1 month ago

alecbuetow commented 1 month ago

Can we keep this pull request open for a few hours? I think Github is registering the merge as one commit on the upstream repo but still tracking every individual commit in my forked repo, which is forcing me to re-sync for every merge. I want to check if committing directly to the PR resolves this

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 43.42%. Comparing base (c1c132f) to head (93b708d). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #130 +/- ## ========================================== + Coverage 42.46% 43.42% +0.95% ========================================== Files 50 50 Lines 4182 4182 ========================================== + Hits 1776 1816 +40 + Misses 2406 2366 -40 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

talgalili commented 1 month ago

Sure

On Wed, 30 Oct 2024, 15:02 codecov[bot], @.***> wrote:

Codecov https://app.codecov.io/gh/talgalili/dendextend/pull/130?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tal+Galili Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.13%. Comparing base (c1c132f) https://app.codecov.io/gh/talgalili/dendextend/commit/c1c132f60750bb8f20b867ba02fe04050e58230f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tal+Galili to head (41646fb) https://app.codecov.io/gh/talgalili/dendextend/commit/41646fb88817d7bd314d1413bb8591bc7685fede?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tal+Galili .

Additional details and impacted files

@@ Coverage Diff @@## master #130 +/- ## ==========================================+ Coverage 42.46% 43.13% +0.66%

Files 50 50 Lines 4182 4182 ==========================================+ Hits 1776 1804 +28 + Misses 2406 2378 -28

☔ View full report in Codecov by Sentry https://app.codecov.io/gh/talgalili/dendextend/pull/130?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tal+Galili . 📢 Have feedback on the report? Share it here https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tal+Galili .

— Reply to this email directly, view it on GitHub https://github.com/talgalili/dendextend/pull/130#issuecomment-2448104167, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHOJBVMP5XNZDEM5BCKYTLZ6EUKXAVCNFSM6AAAAABQ4Y4O2KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBYGEYDIMJWG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

talgalili commented 1 month ago

I suggest to not work on unloadNamespace and the code there. I'd suggest keep it to the end of the work. It's best to focus on testing core functions.

alecbuetow commented 1 month ago

Sounds good, done playing with it

talgalili commented 1 month ago

Cool. Please let me know once I can review this PR

On Thu, 31 Oct 2024, 14:53 Alec Buetow, @.***> wrote:

Sounds good, done playing with it

— Reply to this email directly, view it on GitHub https://github.com/talgalili/dendextend/pull/130#issuecomment-2449775296, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHOJBWNKMSNTRDG2H67ZQDZ6IR6BAVCNFSM6AAAAABQ4Y4O2KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBZG43TKMRZGY . You are receiving this because you commented.Message ID: @.***>

talgalili commented 1 month ago

Thanks, merged.

Keep them coming :)

Regarding the size vs linewidth, I opend an issue on this so we won't forget to find a solution for this in the future: https://github.com/talgalili/dendextend/issues/131