From 0ebbd62a4813df31a67e2654d2ad9ac41f9ec322 Mon Sep 17 00:00:00 2001 From: ChrisW09 <50968720+ChrisW09@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:10:05 +0200 Subject: [PATCH] fix(temporal): emit feature names in the order transform produces All three temporal transformers hstack one ``(n_rows, n_features)`` block per lag / statistic / component, so their columns are block-major. They inherited ``BasePreTabTransformer.get_feature_names_out``, which walks ``zip(input_features, self._output_sizes())`` and therefore emits feature-major names. For any input with more than one column the two disagreed: Lag names : ['A_lag0', 'A_lag1', 'B_lag0', 'B_lag1'] Lag row 0 : [1., 100., 0., 0.] # col 1 is B's lag-1, labelled A_lag1 Override ``get_feature_names_out`` in each class to iterate blocks outermost. The existing ``lag{j}`` / ``roll{j}`` / ``cyclic{j}`` suffix scheme is kept deliberately, so single-feature output is byte-identical and no existing test changes. The issue also suggested naming the columns after the statistic and after sin/cos, which is more informative but changes public output names for users whose labels are correct today; that is left as a separate change. Closes #15 Co-Authored-By: Claude Opus 5 (1M context) --- pretab/transformers/temporal/cyclic.py | 18 +++++ pretab/transformers/temporal/lag.py | 20 ++++++ pretab/transformers/temporal/rolling_stats.py | 21 ++++++ tests/test_temporal.py | 70 +++++++++++++++++++ 4 files changed, 129 insertions(+) diff --git a/pretab/transformers/temporal/cyclic.py b/pretab/transformers/temporal/cyclic.py index bd8d87c..4368ae3 100644 --- a/pretab/transformers/temporal/cyclic.py +++ b/pretab/transformers/temporal/cyclic.py @@ -65,3 +65,21 @@ def transform(self, X): def _output_sizes(self) -> list[int]: return [2] * self.n_features_in_ + + def get_feature_names_out(self, input_features=None): + """Return output names in the component-major order ``transform`` produces. + + ``transform`` returns ``hstack([sin, cos])``, each an + ``(n_rows, n_features)`` block, so the columns run + ``sin_f0, sin_f1, cos_f0, cos_f1``. The inherited feature-major default + would label them the other way round. The ``cyclic{j}`` suffix scheme is + unchanged, so single-feature output is byte-identical; ``j`` is ``0`` for + the sine component and ``1`` for the cosine. + """ + check_is_fitted(self, "n_features_in_") + if input_features is None: + input_features = [f"x{i}" for i in range(self.n_features_in_)] + return np.asarray( + [f"{feature}_cyclic{j}" for j in range(2) for feature in input_features], + dtype=object, + ) diff --git a/pretab/transformers/temporal/lag.py b/pretab/transformers/temporal/lag.py index 68eda95..cd1000e 100644 --- a/pretab/transformers/temporal/lag.py +++ b/pretab/transformers/temporal/lag.py @@ -63,3 +63,23 @@ def transform(self, X): def _output_sizes(self) -> list[int]: return [self.n_lags] * self.n_features_in_ + + def get_feature_names_out(self, input_features=None): + """Return output names in the lag-major order ``transform`` produces. + + ``transform`` hstacks one ``(n_rows, n_features)`` block per lag, so the + columns run ``lag1_f0, lag1_f1, ..., lag2_f0, ...``. The inherited + feature-major default would label them the other way round, mislabelling + every column but the first and last for multi-feature input. + """ + check_is_fitted(self, "n_features_in_") + if input_features is None: + input_features = [f"x{i}" for i in range(self.n_features_in_)] + return np.asarray( + [ + f"{feature}_lag{lag}" + for lag in range(self.n_lags) + for feature in input_features + ], + dtype=object, + ) diff --git a/pretab/transformers/temporal/rolling_stats.py b/pretab/transformers/temporal/rolling_stats.py index 0bcbc74..c422803 100644 --- a/pretab/transformers/temporal/rolling_stats.py +++ b/pretab/transformers/temporal/rolling_stats.py @@ -84,3 +84,24 @@ def transform(self, X): def _output_sizes(self) -> list[int]: return [len(self.stats)] * self.n_features_in_ + + def get_feature_names_out(self, input_features=None): + """Return output names in the stat-major order ``transform`` produces. + + ``transform`` hstacks one ``(n_rows, n_features)`` block per statistic, + so the columns run ``mean_f0, mean_f1, ..., std_f0, ...``. The inherited + feature-major default would label them the other way round. The + ``roll{j}`` suffix scheme is unchanged, so single-feature output is + byte-identical; ``j`` indexes ``self.stats``. + """ + check_is_fitted(self, "n_features_in_") + if input_features is None: + input_features = [f"x{i}" for i in range(self.n_features_in_)] + return np.asarray( + [ + f"{feature}_roll{j}" + for j in range(len(self.stats)) + for feature in input_features + ], + dtype=object, + ) diff --git a/tests/test_temporal.py b/tests/test_temporal.py index 5a136c7..fae1f4f 100644 --- a/tests/test_temporal.py +++ b/tests/test_temporal.py @@ -117,3 +117,73 @@ def test_cyclic_feature_names(): np.testing.assert_array_equal( transformer.get_feature_names_out(["hour"]), ["hour_cyclic0", "hour_cyclic1"] ) + + +# --------------------------------------------------------------------------- # +# Output names must line up with the columns ``transform`` actually produces. +# +# All three transformers hstack one (n_rows, n_features) block per lag / stat / +# component, so the columns are block-major. The inherited default in +# ``BasePreTabTransformer`` emits feature-major names, which mislabelled every +# column but the first and last whenever there was more than one input feature. +# --------------------------------------------------------------------------- # +@pytest.fixture +def two_features(): + # B is always 100x A, so each column is identifiable from its value alone. + return np.column_stack([np.arange(8.0), np.arange(8.0) * 100]) + + +def test_lag_names_match_column_order(two_features): + transformer = LagFeatureTransformer(n_lags=2).fit(two_features) + + names = list(transformer.get_feature_names_out(["A", "B"])) + assert names == ["A_lag0", "B_lag0", "A_lag1", "B_lag1"] + + row = transformer.transform(two_features)[0] + # Column 1 holds B's lag-1 value (100), so it must be named for B. + assert row[1] == 100.0 + assert names[1].startswith("B") + + +def test_rolling_names_match_column_order(two_features): + transformer = RollingStatsTransformer(window_size=3, stats=("mean", "max")).fit(two_features) + + names = list(transformer.get_feature_names_out(["A", "B"])) + assert names == ["A_roll0", "B_roll0", "A_roll1", "B_roll1"] + + row = transformer.transform(two_features)[0] + assert row.tolist() == [1.0, 100.0, 2.0, 200.0] + + +def test_cyclic_names_match_column_order(): + X = np.column_stack([np.arange(8.0), np.arange(8.0)]) + transformer = CyclicalTimeTransformer(period=8).fit(X) + + names = list(transformer.get_feature_names_out(["A", "B"])) + assert names == ["A_cyclic0", "B_cyclic0", "A_cyclic1", "B_cyclic1"] + + out = transformer.transform(X) + np.testing.assert_allclose(out[:, :2], np.sin(2 * np.pi * X / 8)) + np.testing.assert_allclose(out[:, 2:], np.cos(2 * np.pi * X / 8)) + + +@pytest.mark.parametrize( + ("transformer", "expected"), + [ + (LagFeatureTransformer(n_lags=2), ["x0_lag0", "x0_lag1"]), + (RollingStatsTransformer(window_size=3, stats=("mean",)), ["x0_roll0"]), + (CyclicalTimeTransformer(period=8), ["x0_cyclic0", "x0_cyclic1"]), + ], +) +def test_default_names_are_generated_for_single_feature(transformer, expected): + X = np.arange(8.0).reshape(-1, 1) + assert list(transformer.fit(X).get_feature_names_out()) == expected + + +def test_names_count_matches_transform_width(two_features): + for transformer in ( + LagFeatureTransformer(n_lags=3), + RollingStatsTransformer(window_size=3, stats=("mean", "std", "min")), + ): + transformer.fit(two_features) + assert len(transformer.get_feature_names_out()) == transformer.transform(two_features).shape[1]