Repository navigation
silhouette_samples(metric='precomputed') silently ignores metric_params, n_jobs, verbose #714
Description
Problem
tslearn.clustering.silhouette_samples(X, labels, metric="precomputed", metric_params=..., n_jobs=..., verbose=...) silently drops all of metric_params, **kwds, n_jobs, and verbose. The docstring promises metric_params and **kwds are passed to the distance function, and silhouette_score raises on unexpected kwargs via sklearn forwarding, so the silent no-op is a contract inconsistency on the new API surface.
Suggested fix (design call)
Either (a) raise TypeError for any non-None metric_params/n_jobs/verbose/**kwds on the precomputed branch (loud rejection, parity with silhouette_score), or (b) explicitly document that these are ignored when metric == "precomputed".
Metadata
Severity: P3
Confidence: 75
Reviewer(s): api-contract, adversarial (ce-code-review run 20260821-221628-4e205155 on PR #703)
Finding ID: tslearn/clustering/utils.py:330 silhouette_samples(metric="precomputed") silently drops metric_params
Activity
charavelg commentedon Aug 26, 2026
Neither tslearn silhouette_score or sklearn one raise on unexpected kwargs with metric=precomputed. When precomputed is used, no distance function is used.
ApparentlyTejas commentedon Aug 26, 2026
I'd like to take this. Looking at the code,
silhouette_samples(metric="precomputed")returns early beforemetric_params/n_jobs/verbose/**kwdsare ever used, so they're silently dropped — no distance function runs on a precomputed matrix, so there's nothing for them to configure. This matches sklearn's ownsilhouette_samples(metric="precomputed"), which behaves the same way (per the discussion on #713).Given that, I'm going with option (b): document the ignored behavior rather than raising, for parity with sklearn and with
silhouette_score's existing precomputed handling. Concretely:metric_params,n_jobs,verbose, and**kwdsstating they're ignored whenmetric="precomputed".metric_params/n_jobs/verbosealongsidemetric="precomputed"doesn't change the result.Opening a PR shortly.