feat: add hierarchical clustering analytics - #1644
Open
hidekoji wants to merge 6 commits into
Open
Conversation
kei51e
approved these changes
Aug 31, 2026
…157) All four were found by the spec-driven harness added on the tam side (src/test/analytics-harness/specs/hclust.json), by writing down what the analytics SHOULD do and then measuring. 1. A selected variable with no finite value made complete.cases() drop EVERY row, and the failure surfaced as "At least two valid rows are required for hierarchical clustering." -- blaming the rows for one unusable column, and naming neither. Such a column is now dropped with a warning that names it, so the analytics still runs on the variables that do carry data, the way K-Means already does. If nothing usable is left, the error names the columns instead of the rows. 2. The constant-variable error did not say WHICH variable was constant, leaving the user to find it by trial and error with several selected. It now names them. 3. .hclust_merge_distance bounded its table by max_centers -- how far the elbow/silhouette DIAGNOSTIC sweeps over k -- instead of max_interactive_k, how far the dendrogram widget's slider goes. The table exists to justify a cluster count picked with that slider, and cuts_json already used max_interactive_k, so the two disagreed: at max_interactive_k = 2 the slider offered one cut while the table listed nine, six of them unreachable. 4. The per-row silhouette of the chosen cut was computed only when elbow_method_mode was "silhouette". That setting selects which SWEEP runs over k; it says nothing about whether the quality of the cut the user actually chose should be reported, and the Cluster Summary that reports it is not gated on the mode -- so switching the diagnostic to Elbow or None blanked three of its columns. Two different things shared one branch; the sweep (silhouette_result) stays gated, the per-row values (silhouette_values) are hoisted out. The cost is unchanged: it is one silhouette over at most silhouette_sample_size rows, which silhouette mode already paid. Six regression tests, each failing before its fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dendrogram showed row numbers on its leaves because .hclust_build_nodes -- which has always taken `labels` as its own argument alongside `row_ids` -- was being handed the row ids twice. exp_hclust now accepts an optional label_col whose values become those labels. It takes no part in the clustering. The labels are read from the same frame AFTER any sampling and sliced by the same `valid` mask as row_ids, which is the whole difficulty here: a label list built from the unfiltered data still produces a full, plausible-looking tree with every name on the wrong leaf, and raises nothing. Sabotaging exactly that -- slicing by seq_along(row_ids) instead of `valid` -- moved 44 of 46 labels, and only the case WITH excluded rows noticed. Three tests: labels aligned to their own source row both plainly and with rows excluded, the clustering unchanged with and without a label column, and an unknown column rejected by name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
exp_hclust()usingstats::dist()and a singlefastcluster::hclust()fit.Validation
testthat::test_file("tests/testthat/test_hclust.R")— passtestthat::test_file("tests/testthat/test_kmedoids.R")— pass (one existing all-tied PCoA warning)R CMD INSTALL— passR CMD check --no-manual --no-vignettes .— blocked before package checks by the repository metadata errorRequired fields missing or empty: Author, Maintainerunder R 4.6.1.Pairs with the tam-side
fix/issue-38157-spec-design-53f97fimplementation.