fourier_features discarded the series it was handed

src/ml4t/engineer/features/ml/fourier_features.py

@@ -1,3 +1,5 @@
+import warnings
+
 import numpy as np
 import polars as pl
 
@@ -10,33 +12,50 @@
 @feature(
     name="fourier_features",
     category="ml",
-    description="Fourier Features - spectral features for ML",
+    description="Fourier Features - a deterministic seasonal basis on row position",
     normalized=False,
     formula="",
     ta_lib_compatible=False,
 )
 def fourier_features(
-    close: pl.Expr | str,
+    close: pl.Expr | str | None = None,
     n_components: int = 10,
     period: int | None = None,
 ) -> dict[str, pl.Expr]:
-    """Extract Fourier features for capturing periodic patterns.
+    """Build a deterministic seasonal basis: sin and cos of row position.
+
+    These are the Fourier *seasonality terms* used to let a model represent a cycle of a
+    known length, not a spectral transform of a series. **No price or other data enters
+    them.** Feature ``k`` is ``sin(2 pi k t / period)`` and ``cos(2 pi k t / period)``,
+    where ``t`` is the row index, so the output depends only on how many rows there are
+    and on ``period``.
+
+    Two consequences worth stating, because both have been misread:
 
-    Useful for capturing complex seasonal patterns in price data.
+    - The basis is keyed on **row position, not on a timestamp.** It describes the cycle
+      you asked for only where rows are sorted and regularly spaced with no gaps. Across a
+      weekend, a holiday, or a filtered panel, position and time part company.
+    - ``period`` is measured in **rows**, not in minutes. The 390 default is one US equity
+      session counted in one-minute bars; on daily bars it describes a 390-session cycle,
+      which is almost certainly not what the caller meant. Pass the period explicitly.
 
     Parameters
     ----------
-    close : pl.Expr | str
-        Time series column
+    close : pl.Expr | str, optional
+        **Deprecated and unused.** Accepted so existing callers keep working. The
+        function never read it: the previous implementation evaluated
+        ``pl.col(close) if isinstance(close, str) else close`` as a bare statement and
+        discarded the result, so every version of this function has returned a basis
+        independent of the series it was handed. Passing it now warns.
     n_components : int, default 10
-        Number of Fourier components
+        Number of harmonics. Component ``k`` completes ``k`` cycles per ``period`` rows.
     period : int, optional
-        Base period (if None, assumes daily = 390 minutes for US markets)
+        Cycle length **in rows**. Defaults to 390 with a warning; see above.
 
     Returns
     -------
     dict[str, pl.Expr]
-        Dictionary of Fourier features
+        ``fourier_sin_k`` and ``fourier_cos_k`` for k in 1..n_components.
 
     Raises
     ------
@@ -50,14 +69,28 @@ def fourier_features(
     if period is not None:
         validate_window(period, min_window=1, name="period")
 
-    pl.col(close) if isinstance(close, str) else close
+    if close is not None:
+        warnings.warn(
+            "fourier_features() does not read `close` and never has: it returns a "
+            "deterministic seasonal basis on row position. Drop the argument, and if you "
+            "wanted spectral content of the series, this is not the function for it.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
 
     if period is None:
-        period = 390  # Trading minutes in a day
+        period = 390  # one US equity session in one-minute bars
+        warnings.warn(
+            "fourier_features() period defaulted to 390 rows, which is one US equity "
+            "session counted in one-minute bars. On any other bar size that is a cycle "
+            "nobody asked for - on daily bars it is 390 sessions. Pass `period` explicitly.",
+            UserWarning,
+            stacklevel=2,
+        )
 
     features = {}
 
-    # Create time index (assumes sequential data)
+    # Row position. Regular spacing is the caller's to guarantee; see the note above.
     t = pl.int_range(pl.len()).cast(pl.Float64)
 
     for k in range(1, n_components + 1):

tests/test_ml_features.py

@@ -213,8 +213,10 @@ def test_volatility_adjusted_returns(self, sample_data):
 
     def test_fourier_features(self, sample_data):
         """Test Fourier feature extraction."""
-        # Test with default period
-        fourier_default = fourier_features("close", n_components=3)
+        # Test with default period. `close` is deprecated and unused; the warning is the
+        # point of the separate test below.
+        with pytest.warns((DeprecationWarning, UserWarning)):
+            fourier_default = fourier_features("close", n_components=3)
 
         result = sample_data.with_columns(
             [fourier_default[key].alias(key) for key in fourier_default],
@@ -231,10 +233,56 @@ def test_fourier_features(self, sample_data):
             assert (result[col] <= 1).all()
 
         # Test with custom period
-        fourier_custom = fourier_features("close", n_components=2, period=100)
+        with pytest.warns(DeprecationWarning):
+            fourier_custom = fourier_features("close", n_components=2, period=100)
 
         assert len(fourier_custom) == 4  # 2 components * 2 (sin, cos)
 
+    def test_fourier_features_do_not_depend_on_any_series(self, sample_data):
+        """The basis is a function of row position and period only.
+
+        This is what the previous test could not see. It checked names and the [-1, 1]
+        bound, both of which hold for a basis that ignores its input - and the
+        implementation did ignore it, evaluating `pl.col(close) if isinstance(close, str)
+        else close` as a bare statement and discarding it. Naming the property directly
+        means the docstring and the code now have to agree.
+        """
+        basis = fourier_features(n_components=2, period=64)
+        scrambled = sample_data.with_columns(
+            pl.col("close").reverse().alias("close"),
+            (pl.col("feature1") * -100.0).alias("feature1"),
+        )
+        original = sample_data.with_columns([basis[k].alias(k) for k in basis])
+        altered = scrambled.with_columns([basis[k].alias(k) for k in basis])
+        for col in basis:
+            assert original[col].to_list() == altered[col].to_list()
+
+    def test_fourier_features_period_is_measured_in_rows(self, sample_data):
+        """Component k completes exactly k cycles per `period` rows.
+
+        Pinning the period to rows is what makes the 390 default legible as a trap: it is
+        one US equity session in one-minute bars, and 390 sessions on daily bars.
+        """
+        import numpy as np
+
+        period = 64
+        basis = fourier_features(n_components=1, period=period)
+        got = sample_data.select(basis["fourier_sin_1"].alias("s"))["s"].to_list()
+        t = np.arange(len(got), dtype=float)
+        expected = np.sin(2 * np.pi * t / period)
+        assert np.allclose(got, expected)
+        # One full cycle later, the basis repeats.
+        assert got[0] == pytest.approx(got[period], abs=1e-9)
+
+    def test_fourier_features_warns_when_the_period_is_defaulted(self):
+        """390 is one US equity session in minute bars and wrong at any other bar size."""
+        with pytest.warns(UserWarning, match="390 rows"):
+            fourier_features(n_components=1)
+
+    def test_fourier_features_warns_when_handed_a_series(self):
+        with pytest.warns(DeprecationWarning, match="does not read"):
+            fourier_features("close", n_components=1, period=64)
+
     def test_interaction_features(self, sample_data):
         """Test polynomial interaction feature creation."""
         # Test degree 2 interactions