Skip to content

[BUG] keep samples and groups in MultiNormalizer.fit_transform - #2473

Open
bercedev wants to merge 1 commit into
sktime:mainfrom
bercedev:fix-multinormalizer-fit-transform
Open

bercedev wants to merge 1 commit into
sktime:mainfrom
bercedev:fix-multinormalizer-fit-transform

Conversation

@bercedev

Copy link
Copy Markdown

Reference Issues/PRs

Fixes #2472.

What does this implement/fix? Explain your changes.

MultiNormalizer had no fit_transform of its own, so it used sklearn's TransformerMixin.fit_transform, i.e. self.fit(y).transform(y). That breaks in two ways:

  • fit reads one column per target (y[:, idx]), but for ndarray and Tensor input transform reads one row per target (y[idx]), and only transposes a DataFrame itself. So fit_transform on an array or tensor scaled the first samples instead of the targets, and returned arrays with as many elements as there are targets.
  • X never reached transform, so with a GroupNormalizer among the normalizers the call failed with either target_scale or X has to be passed.

This adds MultiNormalizer.fit_transform(y, X=None, return_norm=False). It fits on y as fit expects, transposes an array or tensor for transform (a DataFrame is left as it is), and passes X and return_norm on. The result is the same as the explicit fit(y).transform(y.T) control in the issue. Nothing in the library calls MultiNormalizer.fit_transform (TimeSeriesDataSet uses transform), so existing code paths don't change.

What should a reviewer concentrate their feedback on?

  • Whether fit_transform should keep the existing transform layout (one row per target for arrays/tensors), as done here, rather than changing transform.

Did you add any tests for the change?

Yes, in tests/test_data/test_encoders.py:

  • test_MultiNormalizer_fit_transform[numpy|torch|pandas]: every target keeps all 4 samples and equals a TorchNormalizer fitted on that target alone. numpy and torch fail on main, pandas already passed.
  • test_MultiNormalizer_fit_transform_passes_groups: with a GroupNormalizer, the first target equals GroupNormalizer.fit_transform(data["a"], data). Fails on main with the assertion above.

tests/test_data/test_encoders.py: 62 passed (Python 3.12, CPU torch 2.14.1). ruff 0.6.9 (the pre-commit version) check and format are clean.

Any other comments?

Note: I used Claude Code while working on this.

PR checklist

  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG].
  • Added/modified tests
  • Used pre-commit hooks when committing (I ran the ruff hooks' version by hand, not pre-commit run)

MultiNormalizer inherited sklearn's fit_transform, which fitted on one column per target and then transformed the same array, although transform takes one row per target for arrays and tensors, and it never passed X on. Fixes sktime#2472.
@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@ca70dc8). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2473   +/-   ##
=======================================
  Coverage        ?   88.82%           
=======================================
  Files           ?      218           
  Lines           ?    11675           
  Branches        ?        0           
=======================================
  Hits            ?    10370           
  Misses          ?     1305           
  Partials        ?        0           
Flag Coverage Δ
cpu 88.82% <100.00%> (?)
pytest 88.82% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MultiNormalizer.fit_transform drops array samples and loses group data

1 participant