[ENH] Add capability:predict tag for Clustering#3613
Conversation
Adds a capability:predict clustering tag (default True) indicating whether a clusterer supports out-of-sample prediction after fitting. Transductive clusterers set it to False: fit(X)/labels_ and fit_predict(X) work, while predict(X) raises a clear NotImplementedError instead of silently reclustering. - BaseClusterer.predict/predict_proba raise for capability:predict False; fit_predict documented as fit(X) returning labels_, and now passes y through to fit. - Common estimator checks skip predict/predict_proba for clusterers without the capability, and check_clusterer_output instead asserts the clear error plus working fit_predict/labels_. - Adds TimeSeriesAgglomerative (from PR #3553) as the first transductive clusterer, with capability:predict False. - Adds MockTransductiveCluster and base class tests for the new behaviour. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes TimeSeriesAgglomerative, which belongs to PR #3553, keeping only the capability:predict tag, base class behaviour, common check updates and mocks/tests. MockTransductiveCluster is added to the check_estimator tests so the transductive path of the common checks stays exercised until a real transductive clusterer lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Thank you for contributing to
|
Addresses review on #3613: - rewrite the fit_predict tests around the actual contract (fit_predict returns labels_ from the fit it performs, and never routes through _predict), splitting return-value and transductive-validity concerns; - drop the redundant _CountingClusterer in favour of the transductive mock, whose _predict raises, so fit_predict completing proves _predict was never called; - match only the distinctive part of the predict-capability error message; - add docstrings explaining what each test verifies. Coverage of clustering/base.py unchanged (94%; same two n_clusters branches uncovered as before). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a clusterer mock exposing a configurable n_clusters attribute to exercise both remaining branches of the base _predict_proba: sizing the probability matrix from n_clusters, and the max(_predict)+1 fallback when n_clusters is None. clustering/base.py is now at 100% coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
so my bot say this below and has rewritten the test, hopefully more to your liking, Are you ok with the actual changes @patrickzib? Its a design issue more than a testing style issue Good questions, and you're right the test was loosely defined. The contract is that fit_predict(X) is fit(X) then return labels_, so it always refits — and for a non-deterministic clusterer (k-means being the obvious case) two separate fits need not agree, which is exactly why comparing two separate instances was fragile. I've rewritten it to test the contract directly on a single instance: fit_predict(X) returns labels_ from the fit it just performed, never predict(X). MockTransductiveCluster makes it discriminating — its labels_ are a known alternating 0/1 pattern and its predict raises, so matching that pattern shows fit_predict returned labels rather than routing through predict. Agreed the _CountingClusterer was unnecessary. The property under test is the one documented on fit_predict: it must not route through _predict, so it stays valid for transductive clusterers. I've removed the counting clusterer and now test that directly with MockTransductiveCluster, whose _predict raises — fit_predict completing instead of erroring proves _predict was never called. That's the whole test now, with a docstring saying exactly that. What changed in test_base.py
|
Love it that agents are so affirmative :) Thank you. I'll check again this afternoon. |
* ENH add time series agglomerative clusterer * [MAINT] fix fail doctest * Add test_agglomerativeClustering * Add TimeSeriesAgglomerative validation tests * Delete unnecessary files * add distance_threshold < 0 * test: exclude TimeSeriesAgglomerative pending #3613
Motivation
many clusterers have no default use for fittting on unseen data (transducive clusterers). scikit simply has no predict method for these. We need to work it a bit more because of the predict/_predict model. This PR introduces a tag and tests for it in the base class. Needed for #3553
see #1241
What was changed:
transductive and only fit(X)/labels_ and fit_predict(X) work.
_check_predict_capability() helper after the fitted-state check and raise NotImplementedError with your suggested message when the tag is False. fit_predict now passes y through
to fit and its docstring says it is equivalent to fitting on X and returning labels_ — it never touches predict. The fitted-state check deliberately runs before the capability
check so unfitted estimators still raise the standard NotFittedError that common tests expect.
match the base-class contract. test_base.py covers fit_predict/labels_ equivalence, fit_predict independence from _predict (via a call-counting clusterer), the error for tag-False
clusterers, unchanged behaviour for tag-True ones, and the preserved NotFittedError. test_agglomerative.py keeps the PR's sklearn-equivalence tests and adds fit(X).labels_,
fit_predict determinism, and the predict/predict_proba error tests.
Verification: the full check_estimator suite passes for TimeSeriesAgglomerative, DummyClusterer, and TimeSeriesKMeans; the whole clustering module (575 tests, deep learning
excluded), aeon/utils (556 tests), estimator-checking/mock tests (304), and the new class's docstring example all pass locally.