Repository: sktime/sktime
Pull request: [BUG] MAPA forecaster - missing clone, using exogenous data
#### Reference Issues/PRs
Fixes #7822 


#### What does this implement/fix? Explain your changes.
As mentioned in #7822, these changes:
* if a base_forecaster is passed, it should be cloned and not used directly
* base_forecaster should be passed the X, exogenous data

#### Any other comments?
All tests are passing locally for me.
System config:
```
OS Name:	Microsoft Windows 11 Home Single Language
Version:	10.0.26100 Build 26100
```

Problem statement:
[BUG] MAPA forecaster - missing clone, using exogenous data
There are two bugs in the `MAPAforecaster` that should be fixed:

* if a `base_forecaster` is passed, it should be cloned and not used directly
* `base_forecaster` should be passed the `X`, exogenous data

FYI @phoeenniixx

Hints from issue discussion:
I will take this up after I am done with another issue, until then if anyone else wants to go for it feel free!

Commit message: Removed _fh references in code

Patch to review:
diff --git a/sktime/forecasting/mapa.py b/sktime/forecasting/mapa.py
index 2e4c8c39803..793b97a2f2a 100644
--- a/sktime/forecasting/mapa.py
+++ b/sktime/forecasting/mapa.py
@@ -137,7 +137,7 @@ class MAPAForecaster(BaseForecaster):
         "X_inner_mtype": "pd.DataFrame",
         "ignores-exogeneous-X": False,
         "requires-fh-in-fit": True,
-        "authors": ["trnnick", "phoeenniixx"],
+        "authors": ["phoeenniixx", "trnnick", "satvshr"],
         "python_dependencies": "statsmodels",
     }
 
@@ -161,7 +161,7 @@ def __init__(
         self.weights = weights
         self.base_forecaster = base_forecaster
 
-        self._aggregation_levels = (
+        self.aggregation_levels = (
             self.aggregation_levels if self.aggregation_levels else [1, 2, 4]
         )
 
@@ -169,7 +169,6 @@ def __init__(
         self._decomposition_info = {}
         self._y_cols = None
         self._y_name = None
-        self._fh = None
         self._transformation_offset = None
 
         super().__init__()
@@ -177,14 +176,14 @@ def __init__(
         self._base_forecaster = self._initialize_base_forecaster(self.base_forecaster)
 
         if not all(
-            isinstance(level, int) and level > 0 for level in aggregation_levels
+            isinstance(level, int) and level > 0 for level in self.aggregation_levels
         ):
             raise ValueError("All aggregation levels must be positive integers")
 
     def _initialize_base_forecaster(self, base_forecaster):
         """Initialize the base forecaster with appropriate fallbacks."""
         if base_forecaster is not None:
-            return base_forecaster
+            return base_forecaster.clone()
 
         try:
             if _check_soft_dependencies("statsmodels", severity="none"):
@@ -439,13 +438,9 @@ def _fit(self, y, X=None, fh=None):
         self._y_name = y.name if isinstance(y, pd.Series) else None
 
         y = self._ensure_positive_values(y)
-        y = self._handle_missing_data(y)
-
-        if isinstance(y, pd.Series):
-            y = pd.DataFrame(y)
 
         valid_levels = []
-        for level in self._aggregation_levels:
+        for level in self.aggregation_levels:
             try:
                 y_agg = self._aggregate(y, level)
                 y_agg.columns = self._y_cols
@@ -473,7 +468,7 @@ def _fit(self, y, X=None, fh=None):
                 trend_data = decomposed[trend_cols].copy()
                 trend_data.columns = self._y_cols
 
-                forecaster.fit(trend_data)
+                forecaster.fit(trend_data, X=X, fh=fh)
                 self.forecasters[level] = forecaster
                 valid_levels.append(level)
 
@@ -484,7 +479,7 @@ def _fit(self, y, X=None, fh=None):
         if not valid_levels:
             raise ValueError("Failed to fit any aggregation levels")
 
-        self._aggregation_levels = valid_levels
+        self.aggregation_levels = valid_levels
         return self
 
     def _predict(self, fh, X=None):
@@ -504,7 +499,7 @@ def _predict(self, fh, X=None):
         """
         forecasts = []
 
-        for level in self._aggregation_levels:
+        for level in self.aggregation_levels:
             try:
                 info = self._decomposition_info.get(level, {})
                 seasonal_enabled = info.get("seasonal_enabled", False)
@@ -514,7 +509,7 @@ def _predict(self, fh, X=None):
                     warn(f"No forecaster found for level {level}")
                     continue
 
-                forecast = self.forecasters[level].predict(fh)
+                forecast = self.forecasters[level].predict(fh, X)
 
                 if isinstance(forecast, pd.Series):
                     forecast = pd.DataFrame(forecast)
@@ -538,12 +533,10 @@ def _predict(self, fh, X=None):
                     else:  # additive
                         forecast = forecast.add(seasonal_adjustments, axis=0)
 
-                forecast_values = forecast.values
-
-                if forecast_values.ndim == 1:
-                    forecast_values = forecast_values.reshape(-1, 1)
+                if forecast.values.ndim == 1:
+                    forecast.values = forecast.values.reshape(-1, 1)
 
-                forecasts.append(forecast_values.ravel())
+                forecasts.append(forecast.values.ravel())
 
             except Exception as e:
                 warn(f"Failed to generate forecast for level {level}: {str(e)}\n")
@@ -552,7 +545,7 @@ def _predict(self, fh, X=None):
         if not forecasts:
             raise ValueError(
                 "Failed to generate any forecasts. Check the following:\n"
-                f"1. Valid levels: {self._aggregation_levels}\n"
+                f"1. Valid levels: {self.aggregation_levels}\n"
                 f"2. Decomposition info: {self._decomposition_info}\n"
                 f"3. Available forecasters: {list(self.forecasters.keys())}"
             )
@@ -666,18 +659,17 @@ def _update(self, y, X=None, update_params=True):
             y.columns = self._y_cols
 
         y = self._ensure_positive_values(y)
-        y = self._handle_missing_data(y)
 
-        for level in self._aggregation_levels:
+        for level in self.aggregation_levels:
             try:
                 y_agg = self._aggregate(y, level)
                 y_agg.columns = self._y_cols
 
                 if update_params:
                     if hasattr(self.forecasters[level], "update"):
-                        self.forecasters[level].update(y_agg, update_params=True)
+                        self.forecasters[level].update(y_agg, X=X, update_params=True)
                     else:
-                        self.forecasters[level].fit(y_agg)
+                        self.forecasters[level].fit(y_agg, X=X)
 
             except Exception as e:
                 warn(f"Failed to update level {level}: {str(e)}")
@@ -700,7 +692,6 @@ def get_test_params(cls, parameter_set="default"):
         params : dict or list of dict
         """
         from sktime.forecasting.naive import NaiveForecaster
-        from sktime.forecasting.trend import PolynomialTrendForecaster
 
         params = [
             {
@@ -710,14 +701,7 @@ def get_test_params(cls, parameter_set="default"):
                 "decompose_type": "multiplicative",
                 "forecast_combine": "mean",
             },
-            {
-                "aggregation_levels": [1, 3, 6],
-                "base_forecaster": PolynomialTrendForecaster(degree=2),
-                "imputation_method": "ffill",
-                "decompose_type": "multiplicative",
-                "forecast_combine": "weighted_mean",
-                "weights": [0.5, 0.3, 0.2],
-            },
+            {},
         ]
         if _check_soft_dependencies("statsmodels", severity="none"):
             from sktime.forecasting.exp_smoothing import ExponentialSmoothing


Code review comments:
diff_hunk: @@ -161,30 +161,29 @@ def __init__(
         self.weights = weights
         self.base_forecaster = base_forecaster
 
-        self._aggregation_levels = (
+        self.aggregation_levels = (
path: sktime/forecasting/mapa.py
text: @user1:
You cannot change the `self.params`, See the extension template

https://github.com/sktime/sktime/blob/cc92e8053da45b20ce673aaf2e3bbc4ed5bbcf2c/extension_templates/forecasting_simple.py#L137-L138

@author:
Will change that! Had asked you this on discord and made the change based on that but it was probably miscommunication.