Clustering.jl
Clustering.jl copied to clipboard
JuliaStats
fixed ARI (fixes #225 & #226)
- added pair confusion matrix (CM) calculations
- fixed ARI calculation using CM
Codecov Report
Base: 95.18% // Head: 95.12% // Decreases project coverage by -0.05% :warning:
Coverage data is based on head (
fc57919) compared to base (82821e8). Patch coverage: 90.47% of modified lines in pull request are covered.
:exclamation: Current head fc57919 differs from pull request most recent head 945c537. Consider uploading reports for the commit 945c537 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## master #227 +/- ##
==========================================
- Coverage 95.18% 95.12% -0.06%
==========================================
Files 16 17 +1
Lines 1328 1333 +5
==========================================
+ Hits 1264 1268 +4
- Misses 64 65 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/confusion.jl | 83.33% <83.33%> (ø) |
|
| src/randindex.jl | 100.00% <100.00%> (+5.55%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
As rand behavior changed in recent julia versions, the rework of RNG handling or a test review is required.
This PR passes the relevant tests, can it be merged?
This is an old bug that has had other PRs to fix it: https://github.com/JuliaStats/Clustering.jl/pull/99
Looks like tests are passing except for 32-bit Linux. I wonder if the forced conversion to Int in confusion means that something in randindex overflows when Int is only half as big.
Hm, not sure why that change broke the test. Sorry about that, I did test it locally before suggesting it and it was fine.
