Skip to content

[BUG]: Fix TransformedTargetRegressor update() transformer usage and transformer=None predict handling#1028

Open
direkkakkar319-ops wants to merge 4 commits intosktime:mainfrom
direkkakkar319-ops:TransformedTargetRegressor-DirekKakkar
Open

[BUG]: Fix TransformedTargetRegressor update() transformer usage and transformer=None predict handling#1028
direkkakkar319-ops wants to merge 4 commits intosktime:mainfrom
direkkakkar319-ops:TransformedTargetRegressor-DirekKakkar

Conversation

@direkkakkar319-ops
Copy link
Copy Markdown
Contributor

@direkkakkar319-ops direkkakkar319-ops commented Apr 2, 2026

What does this implement/fix?

This PR fixes a few issues in TransformedTargetRegressor (skpro/regression/compose/_ttr.py) where valid usage could fail at runtime.

  • _update now correctly calls self.transformer_.transform(...) instead of invoking the transformer directly.
  • Added proper handling for transformer=None (the default), which now works consistently across:
    1._predict
  1. _predict_quantiles
  2. _predict_interval
  3. _predict_proba
  • Preserved DataFrame index and column structure when transforming y inside _update.

I also added targeted regression tests in skpro/regression/tests/test_ttr.py to cover:

  • behavior when transformer=None for predict and predict_proba
  • correct application of the transformer during update

Does this introduce a new dependency?

No, no new dependencies are introduced.


What should reviewers focus on?

Feedback would be especially helpful on:

  1. correctness of transformer=None handling across prediction methods
  2. _update behavior with sklearn-style transformers (transform, inverse_transform)
  3. whether preserving index/column structure in the transformed y path is appropriate
  4. whether the added tests sufficiently cover the fixes

Tests

Yes, tests have been added:

  • test_ttr_without_transformer_predicts_and_returns_distribution
  • test_ttr_update_applies_transformer_before_delegating

Additional notes

This is a bugfix-only PR. No API changes and no new dependencies.


PR checklist

For all contributions
  • I've added myself to the [[list of contributors]
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG].
For new estimators
  • I've added the estimator to the API reference (N/A)
  • I've added usage examples to the docstring (N/A)
  • I've handled soft dependencies if any (N/A)

Local Test

Validation (local CI-equivalent checks)
  • pre-commit run --files skpro/regression/compose/_ttr.py skpro/regression/tests/test_ttr.py passed.
  • Local test run passed: 13406 passed, 33 skipped, 468 warnings
Evidence
image image

Copy link
Copy Markdown
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

I think the reason that this did not get caught by the tests is that the get_test_params did have no example where the regressor was capable of update, I think.

Could you add such an example?

@fkiraly fkiraly added bug module:regression probabilistic regression module labels Apr 2, 2026
@direkkakkar319-ops
Copy link
Copy Markdown
Contributor Author

@fkiraly, youu are exactly right on the likely root cause.

@direkkakkar319-ops
Copy link
Copy Markdown
Contributor Author

will update TransformedTrgetRegressor.gt_test_param to include at least one update-capable regressor

@direkkakkar319-ops
Copy link
Copy Markdown
Contributor Author

so, i will be doing it by adding the 3rd params in the TransformedTargetRegressor.get_test_params

@direkkakkar319-ops
Copy link
Copy Markdown
Contributor Author

Changes made
  1. Added an update-capable example to get_test_params as it was asked for
params3 = {"regressor": OnlineRefit(DummyProbaRegressor()), "transformer": None}
return [params1, params2, params3]
  1. made the transformer=None
    as transformer=None keeps the fixture stable while still fulfilling the goal that is to cover update-capable regressor behavior
    i did this change because first the tried the StandardScaler() but that introduced unrelated interval shape issues in generic tests
Screenshot 2026-04-03 004044

@direkkakkar319-ops
Copy link
Copy Markdown
Contributor Author

ran the tests as precommits they are passing
Screenshot 2026-04-03 014212

Screenshot 2026-04-03 014652

@direkkakkar319-ops
Copy link
Copy Markdown
Contributor Author

Hi, @fkiraly the changes you asked are done

@direkkakkar319-ops direkkakkar319-ops marked this pull request as ready for review April 3, 2026 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug module:regression probabilistic regression module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants