Add matched-null validation for cluster counts - #938
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new public helper, test_clusters(), to validate an observed (or user-supplied) cluster count against matched-null reference data generated via matchednull, with a default mclust::Mclust() (BIC) pipeline and support for custom scalar-returning cluster functions.
Changes:
- Introduces
test_clusters()(exported) with roxygen documentation and matched-null integration. - Adds a focused test suite covering custom functions, reproducibility, controls, argument forwarding, and input validation.
- Updates package metadata/documentation (NEWS entry, DESCRIPTION Suggests, WORDLIST, Rd generation, NAMESPACE export).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
R/test_clusters.R |
Adds the new test_clusters() API plus internal validation/standardization helpers. |
tests/testthat/test-test_clusters.R |
Adds test coverage for the new function’s expected behavior and input checks. |
NEWS.md |
Documents the new user-facing function in the changelog. |
NAMESPACE |
Exports test_clusters() as part of the public API. |
man/test_clusters.Rd |
Generated documentation for test_clusters(). |
DESCRIPTION |
Adds matchednull (>= 0.2.1) to Suggests for the new functionality/tests. |
inst/WORDLIST |
Adds “Meng” to the spelling whitelist for the new reference entry. |
Files not reviewed (1)
- man/test_clusters.Rd: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| insight::check_if_installed("mclust") | ||
| mclustBIC <- mclust::mclustBIC | ||
| max_components <- min(as.integer(n_max), nrow(x) - 1L) |
| loo, | ||
| MASS, | ||
| Matrix, | ||
| matchednull (>= 0.2.1), |
|
Thanks, looks good to me! A version bump (4th digits, to increase dev version) in the DESCRIPTION is missing (or overwritten by merge main into this branch), else I have no other comments. @DominiqueMakowski anything you would like to add? |
|
Thanks! I bumped the development version to 0.17.1.11. |
|
This is a bit clunky #' # Any scalar-returning clustering pipeline can be tested.
#' pick_two <- function(data) 2
#' test_clusters(iris[, 1:4], cluster_function = pick_two, iterations = 19)Rather than having the whole Mclust complication (which is used to get a number), why not streamline the api and just have the The pipeline could then be like n <- n_clusters(data)
# this suggests 3
test_clusters(data, n_clusters = c(2, 3, 5))or am i missing somethign |
|
Thanks for pointing that out. I agree that the I’ll replace the example with a real clustering pipeline and clarify this in the documentation. |
|
You mean the function needs to be aware of the clustering method? Then why not (I'm just brainstorming I dont have strong opinions) making the function a method that runs on a cluster_analysis() result? So the workflow would go like |
|
Yes that’s what I mean. I like the idea of making it a method for a The only thing I want to preserve is that the current test evaluates the selection process, rather than just one fixed solution. |
if need be we can add more attributes to store more info :) we do that quite commonly across the easyverse |
Closes easystats/easystats#479.
This adds
test_clusters(), which compares a selected cluster count with counts from matched-null data generated bymatchednull. The default pipeline standardizes the data and usesmclust::Mclust()to select the number of components by BIC; custom scalar-returning clustering functions are also supported.The public API keeps the main controls explicit, while options such as the copula and parallel evaluation can be passed through
...tomatchednull::matched_null_test().I added tests for custom functions, reproducibility, null and positive controls, argument forwarding, and input validation.
lintrpassed