talgalili / dendextend

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

Add error condition for duplicated labels #72

Closed alanocallaghan closed 6 years ago

alanocallaghan commented 6 years ago

As discussed here: https://github.com/talgalili/heatmaply/issues/155

This doesn't alter existing behaviour, just gives a more intelligible error message

codecov-io commented 6 years ago

Codecov Report

Merging #72 into master will increase coverage by 0.53%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   37.92%   38.46%   +0.53%     
==========================================
  Files          51       50       -1     
  Lines        4021     3913     -108     
==========================================
- Hits         1525     1505      -20     
+ Misses       2496     2408      -88
Impacted Files Coverage Δ
R/seriate_dendrogram.R 0% <0%> (ø) :arrow_up:
R/branches_attr_by.R 0% <0%> (-17.9%) :arrow_down:
R/set.dendrogram.R 78.04% <0%> (-1.5%) :arrow_down:
R/colored_bars.R 0% <0%> (ø) :arrow_up:
R/colored_dots.R

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c0885f6...aa355cc. Read the comment docs.

talgalili commented 6 years ago

Hi @Alanocallaghan Thank you for the PR.

This seems to cause an error in one of the tests, is it something you could fix? (I suspect that the test needs to be amended)

alanocallaghan commented 6 years ago

Sure, I'll have another look tomorrow, was just putting it together quickly

talgalili commented 6 years ago

Hi @Alanocallaghan I see Travis reports a failure in the test:

 ══ testthat results  ═══════════════════════════════════════════════════════════
  OK: 358 SKIPPED: 0 FAILED: 1
  1. Failure: seriate fails with duplicated labels (@test-rotate.R#97) 

  Error: testthat unit tests failed

Could you please have a look and fix it?

alanocallaghan commented 6 years ago

I can't figure this out, it works locally (though other tests break instead). Anyway not a major issue so I'll close