feat: reduce public surface and roadmap cleanup (roadmap 2.4, priority 5) - #48
Merged
Merged
Conversation
…y 5) - 2.4: drop predict_all and the operator overloads __add__ / __sub__ / __getitem__; ErrorAnalyzer now uses get_estimator. The named methods already covered the same behavior. - get_results(wrt_dummy=True) now requires exactly one dummy estimator (raises otherwise) and returns NaN stds, since a std ratio is not meaningful; documented as 'mean ratios only'. - get_predictions_similarity excludes dummies by class (isinstance) rather than name substring, so a user-named dummy is still excluded. - document the PONIARD_TQDM_LEAVE env var in the README.
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
1 participant
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
Roadmap sequencing step 8 (final) — 2.4 (reduce the public method surface) and Priority 5 (cleanup).
Changes
2.4 — cut the operator overloads
predict_all(never used),__add__,__sub__,__getitem__fromPoniardBaseEstimator.ErrorAnalyzernow callsget_estimator(...)instead of__getitem__.add_estimators/remove_estimators; the dedicated operator tests were removed (the named methods are already covered).Priority 5 — cleanup
get_results(wrt_dummy=True): now requires exactly one dummy estimator — raises a clearValueErrorotherwise (fixes the silent multiple-Dummy.squeeze()case). Standard deviations are returned asNaN, since a std ratio is not meaningful; the docstring documents this as "mean ratios only."get_predictions_similarity: dummies are excluded by class (isinstance(..., (DummyClassifier, DummyRegressor))) instead of name substring match, so a user-named dummy (e.g."baseline") is correctly excluded.PONIARD_TQDM_LEAVEenv var.Already satisfied by earlier PRs
_fitted_pipeline_idsrebuild invalidation (1.3);poniard_taskdeferred import left as-is per roadmap.Tests
wrt_dummywithstd=Truereturns NaN stds;wrt_dummyraises with two dummies; a user-named dummy is excluded fromget_predictions_similarity.ruffclean.