From 6873c09b8e81b4722fffab35dcf5d894b00cfe74 Mon Sep 17 00:00:00 2001 From: Mohamed Belhsan Hmida Date: Thu, 30 Jul 2026 14:15:55 +0100 Subject: [PATCH 1/7] fix: let forecast-only future regressors reach the training window Future regressors whose sensors only ever record ex-ante beliefs (belief time never after the event start, e.g. day-ahead market fundamentals) were entirely filtered out of the training window by the strict realized-only selection in split_data_all_beliefs, leaving the model a constant, interpolation-filled series. The training-window selection now falls back to the latest forecast per event when no realized belief exists, while still preferring realized values where available. The predict-window slice and past regressors keep their strict semantics. Co-Authored-By: Claude Fable 5 --- .../data/models/forecasting/pipelines/base.py | 27 +++- .../data/tests/test_forecasting_pipeline.py | 118 ++++++++++++++++++ 2 files changed, 142 insertions(+), 3 deletions(-) diff --git a/flexmeasures/data/models/forecasting/pipelines/base.py b/flexmeasures/data/models/forecasting/pipelines/base.py index c860482a24..1d58971c41 100644 --- a/flexmeasures/data/models/forecasting/pipelines/base.py +++ b/flexmeasures/data/models/forecasting/pipelines/base.py @@ -709,17 +709,34 @@ def _latest_known_per_regressor( regressor_columns: list[str], forecast_belief_time: pd.Timestamp, realized_only: bool = False, + fall_back_to_forecast: bool = False, ) -> pd.DataFrame: - """Select latest regressor values known at forecast belief time.""" + """Select latest regressor values known at forecast belief time. + + :param realized_only: Keep only ex-post beliefs + (belief time after the event started); + otherwise, keep only ex-ante beliefs. + :param fall_back_to_forecast: With ``realized_only``, also keep + ex-ante beliefs for events without any + ex-post belief. Some regressor sensors + (e.g. day-ahead market fundamentals) + only ever record ex-ante beliefs, and + would otherwise yield no values at all. + """ keep = ["event_start", *regressor_columns] if df_.empty: return df_.iloc[0:0][keep].copy() known = df_.loc[df_["belief_time"] <= forecast_belief_time].copy() - if realized_only: + if realized_only and not fall_back_to_forecast: known = known.loc[known["belief_time"] > known["event_start"]] - else: + elif not realized_only: known = known.loc[known["belief_time"] <= known["event_start"]] + # With realized_only and fall_back_to_forecast, both ex-post and + # ex-ante beliefs are kept: for any given event, an ex-post belief + # time necessarily exceeds every ex-ante belief time, so selecting + # the latest belief per event prefers realized values and falls + # back to forecasts only where no realized value exists. if known.empty: return df_.iloc[0:0][keep].copy() @@ -840,12 +857,16 @@ def _overlay_annotations( # values would hide it from the training window entirely. Its # visibility is governed solely by its own belief time, applied # below once the sensor-based frame has been assembled. + # Forecast-only sensors (belief time never after the event + # start, e.g. day-ahead fundamentals) have no realized rows, + # so the training window falls back to their latest forecasts. future_regressor_columns = self.future_regressors future_known = _latest_known_per_regressor( X_future_regressors_df, future_regressor_columns, belief_time, realized_only=True, + fall_back_to_forecast=True, ) realized_slice = _slice_closed( future_known, target_start, target_end diff --git a/flexmeasures/data/tests/test_forecasting_pipeline.py b/flexmeasures/data/tests/test_forecasting_pipeline.py index 59dc894616..eb71bff230 100644 --- a/flexmeasures/data/tests/test_forecasting_pipeline.py +++ b/flexmeasures/data/tests/test_forecasting_pipeline.py @@ -1724,6 +1724,124 @@ def capture_frame(self, df, sensors, sensor_names, start, end, **kwargs): assert 77.0 not in set(values_by_event) +def test_forecast_only_future_regressor_populates_training_window(monkeypatch): + """A future regressor may only ever record ex-ante beliefs (e.g. day-ahead + market fundamentals, whose belief time never passes the event start). + The training window must then fall back to the latest forecasts instead of + coming out empty, while realized values still win where they exist. + """ + target_sensor = type( + "SensorStub", + (), + {"name": "target", "id": 1, "event_resolution": timedelta(hours=1)}, + )() + forecast_only_regressor = type( + "SensorStub", + (), + {"name": "residual-load", "id": 2, "event_resolution": timedelta(hours=1)}, + )() + revised_regressor = type( + "SensorStub", + (), + {"name": "weather", "id": 3, "event_resolution": timedelta(hours=1)}, + )() + + pipeline = BasePipeline( + target_sensor=target_sensor, + future_regressors=[forecast_only_regressor, revised_regressor], + past_regressors=[], + n_steps_to_predict=1, + max_forecast_horizon=1, + forecast_frequency=1, + event_starts_after=datetime(2025, 1, 8, 6), + event_ends_before=datetime(2025, 1, 8, 10), + ) + regressor_a, regressor_b = pipeline.future_regressors + + day_ahead = pd.Timedelta(hours=21) + rows = [] + for hour, value_a, value_b in [ + (6, 1.0, 10.0), + (7, 2.0, 20.0), + (8, 3.0, 30.0), + (9, 4.0, 40.0), + (10, 5.0, 50.0), + (11, 6.0, 60.0), + ]: + event_start = pd.Timestamp(f"2025-01-08T{hour:02d}:00:00") + # Day-ahead beliefs only: each belief precedes its event start. + rows.append( + { + "event_start": event_start, + "belief_time": event_start - day_ahead, + pipeline.target: None, + regressor_a: value_a, + regressor_b: value_b, + } + ) + if hour <= 9: + # The target realizes ex post, but neither regressor does here, + # so these rows must not shadow the day-ahead regressor beliefs. + rows.append( + { + "event_start": event_start, + "belief_time": event_start + pd.Timedelta(minutes=30), + pipeline.target: 100.0 + hour, + regressor_a: None, + regressor_b: None, + } + ) + # Regressor B alone gets one realized revision within the training window. + rows.append( + { + "event_start": pd.Timestamp("2025-01-08T07:00:00"), + "belief_time": pd.Timestamp("2025-01-08T08:00:00"), + pipeline.target: None, + regressor_a: None, + regressor_b: 25.0, + } + ) + df = pd.DataFrame(rows) + + captured_future_frames = [] + + # Capture the covariate frame before missing-value filling converts it + # to a Darts TimeSeries. This keeps the test focused on in-memory belief + # selection instead of requiring database-backed sensor data. + def capture_frame(self, df, sensors, sensor_names, start, end, **kwargs): + if sensor_names == self.future_regressors: + captured_future_frames.append(df.copy()) + return df + + monkeypatch.setattr(BasePipeline, "detect_and_fill_missing_values", capture_frame) + + pipeline.split_data_all_beliefs(df) + + assert len(captured_future_frames) == 1, ( + "Expected one future-covariate frame because this one-step pipeline " + "prepares exactly one split." + ) + selected = captured_future_frames[0].set_index("event_start") + for hour, expected in [(6, 1.0), (7, 2.0), (8, 3.0), (9, 4.0)]: + assert ( + selected.loc[pd.Timestamp(f"2025-01-08T{hour:02d}:00:00"), regressor_a] + == expected + ), ( + "Expected the forecast-only regressor's day-ahead values to fill " + "the training window, because no realized beliefs exist to prefer." + ) + assert selected.loc[pd.Timestamp("2025-01-08T10:00:00"), regressor_a] == 5.0 + assert selected.loc[pd.Timestamp("2025-01-08T11:00:00"), regressor_a] == 6.0 + assert selected.loc[pd.Timestamp("2025-01-08T07:00:00"), regressor_b] == 25.0, ( + "Expected the realized revision to win over the day-ahead belief for " + "the same event, because its belief time is necessarily later." + ) + assert selected.loc[pd.Timestamp("2025-01-08T06:00:00"), regressor_b] == 10.0, ( + "Expected the day-ahead fall-back to apply per event, so a realized " + "revision for one event does not affect its neighbours." + ) + + def test_annotation_regressor_split_preserves_annotation_columns(monkeypatch): target_sensor = type( "SensorStub", From 8c1378a35673a66816f18a803a168552f5296f03 Mon Sep 17 00:00:00 2001 From: Mohamed Belhsan Hmida Date: Thu, 30 Jul 2026 15:04:48 +0100 Subject: [PATCH 2/7] fix: parse dict-typed CLI options like --model-params as JSON add_cli_options_from_schema wired Click-level JSON parsing for MarshmallowClickMixin and list fields, but plain fields.Dict options fell through as raw strings, so --model-params '{"max_depth": 6}' failed schema validation with 'Not a valid mapping type'. Dict fields now reuse NestedDictParamType, accepting both JSON and Python-literal syntax. The only workaround used to be passing --config with a file. Co-Authored-By: Claude Fable 5 --- flexmeasures/cli/tests/test_utils.py | 31 ++++++++++++++++++++++++++++ flexmeasures/cli/utils.py | 6 +++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/flexmeasures/cli/tests/test_utils.py b/flexmeasures/cli/tests/test_utils.py index 250ab93378..3ab74096fe 100644 --- a/flexmeasures/cli/tests/test_utils.py +++ b/flexmeasures/cli/tests/test_utils.py @@ -147,6 +147,37 @@ def cmd(name): assert result.output.endswith("foo\n") +def test_add_cli_options_from_schema_parses_dict_fields(): + """A plain ``fields.Dict`` option (e.g. ``--model-params``) must arrive as a + dict, in both JSON and Python-literal syntax, like nested list fields do. + """ + from flexmeasures.cli.utils import add_cli_options_from_schema + from flexmeasures.data.schemas.forecasting.pipeline import ( + TrainPredictPipelineConfigSchema, + ) + + captured = {} + + @click.command() + @add_cli_options_from_schema(TrainPredictPipelineConfigSchema()) + def cmd(**kwargs): + captured.update(kwargs) + + result = CliRunner().invoke(cmd, ["--model-params", '{"min_child_samples": 5}']) + assert result.exit_code == 0, result.output + assert captured["model_params"] == {"min_child_samples": 5} + # The parsed dict must pass schema validation, which used to reject the + # raw string with "Not a valid mapping type." + config = TrainPredictPipelineConfigSchema().load( + {"model-params": captured["model_params"]} + ) + assert config["model_params"] == {"min_child_samples": 5} + + result = CliRunner().invoke(cmd, ["--model-params", "{'max_depth': 6}"]) + assert result.exit_code == 0, result.output + assert captured["model_params"] == {"max_depth": 6} + + @pytest.mark.xfail( strict=True, raises=RuntimeError, diff --git a/flexmeasures/cli/utils.py b/flexmeasures/cli/utils.py index 02039b8989..3e251dc8c2 100644 --- a/flexmeasures/cli/utils.py +++ b/flexmeasures/cli/utils.py @@ -384,7 +384,8 @@ class NestedDictParamType(click.ParamType): Accepts both JSON double-quoted syntax (``{"key": "value"}``) and Python-literal single-quoted syntax (``{'key': 'value'}``). Used for CLI options whose Marshmallow - field type is ``fields.List(fields.Nested(...))``. + field type is ``fields.List(fields.Nested(...))`` (one dict per occurrence) or + ``fields.Dict`` (a single dict). """ name = "DICT" @@ -510,6 +511,9 @@ def decorator(command): kwargs["type"] = NestedDictParamType() else: kwargs["type"] = str + elif isinstance(field, fields.Dict): + # The value is a single dict string; parse it at the Click level. + kwargs["type"] = NestedDictParamType() command = click.option(*options, **kwargs)(command) From f36a807623419da39edb70c61917cddfe7ce54ad Mon Sep 17 00:00:00 2001 From: Mohamed Belhsan Hmida Date: Thu, 30 Jul 2026 15:05:38 +0100 Subject: [PATCH 3/7] docs: changelog entries for the forecasting fixes PR number placeholders to be filled in once the PR is opened. Co-Authored-By: Claude Fable 5 --- documentation/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/documentation/changelog.rst b/documentation/changelog.rst index d3225f1c01..8452fd09c6 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -72,6 +72,8 @@ Infrastructure / Support Bugfixes ----------- +* Future regressors from sensors that only ever record forecasts (belief time never after the event start, e.g. day-ahead market fundamentals) no longer drop out of the forecasting pipeline's training window entirely; the training window now falls back to the latest forecast per event where no realized belief exists [see `PR #XXXX `_] +* Dict-typed CLI options such as ``flexmeasures add forecasts --model-params`` now parse their JSON (or Python-literal) argument instead of failing schema validation with "Not a valid mapping type" [see `PR #XXXX `_] * Replaying a chart for a past window no longer shows annotations that were only recorded later; annotation searches and the ``chart_annotations`` endpoints can now be scoped by recording (belief) time [see `PR #2367 `_] * Continuing the query-parameter cleanup started in PR #2352: the chart-related endpoints now use ``prior``, ``start``, ``end`` and hyphenated field names, with a new ``duration`` field to derive a missing ``start``/``end``; old spellings keep working as legacy aliases [see `PR #2367 `_] * Scheduling jobs no longer print ``Job ... made schedule.`` before ``scheduler.compute()`` runs (only after a successful schedule) [see `PR #2342 `_] From 689b800b693020490e2d0dea0f594c5f16367ea1 Mon Sep 17 00:00:00 2001 From: Mohamed Belhsan Hmida Date: Fri, 31 Jul 2026 02:01:59 +0100 Subject: [PATCH 4/7] docs: add pr number to changelog Signed-off-by: Mohamed Belhsan Hmida --- documentation/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/changelog.rst b/documentation/changelog.rst index 8452fd09c6..2dc156924a 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -72,7 +72,7 @@ Infrastructure / Support Bugfixes ----------- -* Future regressors from sensors that only ever record forecasts (belief time never after the event start, e.g. day-ahead market fundamentals) no longer drop out of the forecasting pipeline's training window entirely; the training window now falls back to the latest forecast per event where no realized belief exists [see `PR #XXXX `_] +* Future regressors from sensors that only ever record forecasts (belief time never after the event start, e.g. day-ahead market fundamentals) no longer drop out of the forecasting pipeline's training window entirely; the training window now falls back to the latest forecast per event where no realized belief exists [see `PR #2373 `_] * Dict-typed CLI options such as ``flexmeasures add forecasts --model-params`` now parse their JSON (or Python-literal) argument instead of failing schema validation with "Not a valid mapping type" [see `PR #XXXX `_] * Replaying a chart for a past window no longer shows annotations that were only recorded later; annotation searches and the ``chart_annotations`` endpoints can now be scoped by recording (belief) time [see `PR #2367 `_] * Continuing the query-parameter cleanup started in PR #2352: the chart-related endpoints now use ``prior``, ``start``, ``end`` and hyphenated field names, with a new ``duration`` field to derive a missing ``start``/``end``; old spellings keep working as legacy aliases [see `PR #2367 `_] From 4a6df4b8be171bfa3a5f4662148ae16a75d4c5d0 Mon Sep 17 00:00:00 2001 From: Mohamed Belhsan Hmida Date: Fri, 7 Aug 2026 15:43:37 +0100 Subject: [PATCH 5/7] docs/changelog: reference PR #2373 for the dict-option fix The entry still carried the #XXXX placeholder that the companion entry above it had already been given a real number for. Co-Authored-By: Claude Opus 5 Signed-off-by: Mohamed Belhsan Hmida --- documentation/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/changelog.rst b/documentation/changelog.rst index 77de903d70..afe8dc902a 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -83,7 +83,7 @@ Infrastructure / Support Bugfixes ----------- * Future regressors from sensors that only ever record forecasts (belief time never after the event start, e.g. day-ahead market fundamentals) no longer drop out of the forecasting pipeline's training window entirely; the training window now falls back to the latest forecast per event where no realized belief exists [see `PR #2373 `_] -* Dict-typed CLI options such as ``flexmeasures add forecasts --model-params`` now parse their JSON (or Python-literal) argument instead of failing schema validation with "Not a valid mapping type" [see `PR #XXXX `_] +* Dict-typed CLI options such as ``flexmeasures add forecasts --model-params`` now parse their JSON (or Python-literal) argument instead of failing schema validation with "Not a valid mapping type" [see `PR #2373 `_] * Show icons for more asset types in the UI's asset structure view, which previously fell back to a question mark: the ``wind``, ``process`` and ``heat-storage`` types that FlexMeasures seeds by default, and EV infrastructure under its various names (such as ``one-way_evse``, ``two-way_evse``, ``evse``, ``charging_station`` and ``charging_hub``) and building services equipment (``hvac``, ``ahu``, ``dhw``, ``heatpump``, ``chiller``, ``lighting`` and ``other-loads``). Asset type names are now matched ignoring case and separators, so an asset type named ``charge-point`` gets the same icon as ``chargepoint`` [see `PR #2391 `_] * Replaying a chart for a past window no longer shows annotations that were only recorded later; annotation searches and the ``chart_annotations`` endpoints can now be scoped by recording (belief) time [see `PR #2367 `_] * Continuing the query-parameter cleanup started in PR #2352: the chart-related endpoints now use ``prior``, ``start``, ``end`` and hyphenated field names, with a new ``duration`` field to derive a missing ``start``/``end``; old spellings keep working as legacy aliases [see `PR #2367 `_] From 3ebd73142b4514887de23f400824f533eb29aa55 Mon Sep 17 00:00:00 2001 From: Mohamed Belhsan Hmida Date: Fri, 7 Aug 2026 15:44:31 +0100 Subject: [PATCH 6/7] style/forecasting: break the new docstrings and comments after punctuation The repo convention is that a docstring or comment line ends at a comma, semicolon, colon or period, so that review comments and text searches stay stable. The prose added for the future-regressor fall-back wrapped mid-phrase in several places. While rewrapping, state explicitly that fall_back_to_forecast is ignored unless realized_only is set, which the branch structure implies but the docstring left to inference. Co-Authored-By: Claude Opus 5 Signed-off-by: Mohamed Belhsan Hmida --- flexmeasures/cli/tests/test_utils.py | 8 +++--- .../data/models/forecasting/pipelines/base.py | 25 ++++++++----------- .../data/tests/test_forecasting_pipeline.py | 14 +++++------ 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/flexmeasures/cli/tests/test_utils.py b/flexmeasures/cli/tests/test_utils.py index 3ab74096fe..485f6cbbf1 100644 --- a/flexmeasures/cli/tests/test_utils.py +++ b/flexmeasures/cli/tests/test_utils.py @@ -148,8 +148,8 @@ def cmd(name): def test_add_cli_options_from_schema_parses_dict_fields(): - """A plain ``fields.Dict`` option (e.g. ``--model-params``) must arrive as a - dict, in both JSON and Python-literal syntax, like nested list fields do. + """A plain ``fields.Dict`` option (e.g. ``--model-params``) must arrive as a dict, + in both JSON and Python-literal syntax, like nested list fields do. """ from flexmeasures.cli.utils import add_cli_options_from_schema from flexmeasures.data.schemas.forecasting.pipeline import ( @@ -166,8 +166,8 @@ def cmd(**kwargs): result = CliRunner().invoke(cmd, ["--model-params", '{"min_child_samples": 5}']) assert result.exit_code == 0, result.output assert captured["model_params"] == {"min_child_samples": 5} - # The parsed dict must pass schema validation, which used to reject the - # raw string with "Not a valid mapping type." + # The parsed dict must pass schema validation, + # which used to reject the raw string with "Not a valid mapping type." config = TrainPredictPipelineConfigSchema().load( {"model-params": captured["model_params"]} ) diff --git a/flexmeasures/data/models/forecasting/pipelines/base.py b/flexmeasures/data/models/forecasting/pipelines/base.py index 1d58971c41..be8a7d5ecf 100644 --- a/flexmeasures/data/models/forecasting/pipelines/base.py +++ b/flexmeasures/data/models/forecasting/pipelines/base.py @@ -713,15 +713,12 @@ def _latest_known_per_regressor( ) -> pd.DataFrame: """Select latest regressor values known at forecast belief time. - :param realized_only: Keep only ex-post beliefs - (belief time after the event started); + :param realized_only: Keep only ex-post beliefs (belief time after the event started); otherwise, keep only ex-ante beliefs. - :param fall_back_to_forecast: With ``realized_only``, also keep - ex-ante beliefs for events without any - ex-post belief. Some regressor sensors - (e.g. day-ahead market fundamentals) - only ever record ex-ante beliefs, and - would otherwise yield no values at all. + :param fall_back_to_forecast: Also keep ex-ante beliefs for events without any ex-post belief, + which is ignored unless ``realized_only`` is set. + Some regressor sensors only ever record ex-ante beliefs (e.g. day-ahead market fundamentals), + and would otherwise yield no values at all. """ keep = ["event_start", *regressor_columns] if df_.empty: @@ -732,11 +729,10 @@ def _latest_known_per_regressor( known = known.loc[known["belief_time"] > known["event_start"]] elif not realized_only: known = known.loc[known["belief_time"] <= known["event_start"]] - # With realized_only and fall_back_to_forecast, both ex-post and - # ex-ante beliefs are kept: for any given event, an ex-post belief - # time necessarily exceeds every ex-ante belief time, so selecting - # the latest belief per event prefers realized values and falls - # back to forecasts only where no realized value exists. + # With realized_only and fall_back_to_forecast, both ex-post and ex-ante beliefs are kept: + # for any given event, an ex-post belief time necessarily exceeds every ex-ante belief time, + # so selecting the latest belief per event prefers realized values, + # and falls back to forecasts only where no realized value exists. if known.empty: return df_.iloc[0:0][keep].copy() @@ -857,8 +853,7 @@ def _overlay_annotations( # values would hide it from the training window entirely. Its # visibility is governed solely by its own belief time, applied # below once the sensor-based frame has been assembled. - # Forecast-only sensors (belief time never after the event - # start, e.g. day-ahead fundamentals) have no realized rows, + # Forecast-only sensors have no realized rows (belief time never after the event start, e.g. day-ahead fundamentals), # so the training window falls back to their latest forecasts. future_regressor_columns = self.future_regressors future_known = _latest_known_per_regressor( diff --git a/flexmeasures/data/tests/test_forecasting_pipeline.py b/flexmeasures/data/tests/test_forecasting_pipeline.py index eb71bff230..9160abb6b6 100644 --- a/flexmeasures/data/tests/test_forecasting_pipeline.py +++ b/flexmeasures/data/tests/test_forecasting_pipeline.py @@ -1725,10 +1725,10 @@ def capture_frame(self, df, sensors, sensor_names, start, end, **kwargs): def test_forecast_only_future_regressor_populates_training_window(monkeypatch): - """A future regressor may only ever record ex-ante beliefs (e.g. day-ahead - market fundamentals, whose belief time never passes the event start). - The training window must then fall back to the latest forecasts instead of - coming out empty, while realized values still win where they exist. + """A future regressor may only ever record ex-ante beliefs (e.g. day-ahead market fundamentals, whose belief time never passes the event start). + + The training window must then fall back to the latest forecasts instead of coming out empty, + while realized values still win where they exist. """ target_sensor = type( "SensorStub", @@ -1805,9 +1805,9 @@ def test_forecast_only_future_regressor_populates_training_window(monkeypatch): captured_future_frames = [] - # Capture the covariate frame before missing-value filling converts it - # to a Darts TimeSeries. This keeps the test focused on in-memory belief - # selection instead of requiring database-backed sensor data. + # Capture the covariate frame before missing-value filling converts it to a Darts TimeSeries. + # This keeps the test focused on in-memory belief selection, + # instead of requiring database-backed sensor data. def capture_frame(self, df, sensors, sensor_names, start, end, **kwargs): if sensor_names == self.future_regressors: captured_future_frames.append(df.copy()) From c22e19c8eb6d19d2c3424f6efe7e1db7d8db8a1a Mon Sep 17 00:00:00 2001 From: Mohamed Belhsan Hmida Date: Fri, 7 Aug 2026 15:44:56 +0100 Subject: [PATCH 7/7] cli: reject a dict-typed option argument that is not a mapping Routing fields.Dict options through NestedDictParamType made --model-params parse, but a parsable non-mapping (a list, a number, a bare string) still travelled on to Marshmallow, which rejected it with "Not a valid mapping type" -- the very message this fix set out to remove. Fail in the parameter type instead, naming what was received. Co-Authored-By: Claude Opus 5 Signed-off-by: Mohamed Belhsan Hmida --- documentation/changelog.rst | 2 +- flexmeasures/cli/tests/test_utils.py | 22 ++++++++++++++++++++++ flexmeasures/cli/utils.py | 13 +++++++++++-- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/documentation/changelog.rst b/documentation/changelog.rst index afe8dc902a..6e9ed349e7 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -83,7 +83,7 @@ Infrastructure / Support Bugfixes ----------- * Future regressors from sensors that only ever record forecasts (belief time never after the event start, e.g. day-ahead market fundamentals) no longer drop out of the forecasting pipeline's training window entirely; the training window now falls back to the latest forecast per event where no realized belief exists [see `PR #2373 `_] -* Dict-typed CLI options such as ``flexmeasures add forecasts --model-params`` now parse their JSON (or Python-literal) argument instead of failing schema validation with "Not a valid mapping type" [see `PR #2373 `_] +* Dict-typed CLI options such as ``flexmeasures add forecasts --model-params`` now parse their JSON (or Python-literal) argument instead of failing schema validation with "Not a valid mapping type"; an argument that parses to something other than a mapping is now reported as such by the option itself [see `PR #2373 `_] * Show icons for more asset types in the UI's asset structure view, which previously fell back to a question mark: the ``wind``, ``process`` and ``heat-storage`` types that FlexMeasures seeds by default, and EV infrastructure under its various names (such as ``one-way_evse``, ``two-way_evse``, ``evse``, ``charging_station`` and ``charging_hub``) and building services equipment (``hvac``, ``ahu``, ``dhw``, ``heatpump``, ``chiller``, ``lighting`` and ``other-loads``). Asset type names are now matched ignoring case and separators, so an asset type named ``charge-point`` gets the same icon as ``chargepoint`` [see `PR #2391 `_] * Replaying a chart for a past window no longer shows annotations that were only recorded later; annotation searches and the ``chart_annotations`` endpoints can now be scoped by recording (belief) time [see `PR #2367 `_] * Continuing the query-parameter cleanup started in PR #2352: the chart-related endpoints now use ``prior``, ``start``, ``end`` and hyphenated field names, with a new ``duration`` field to derive a missing ``start``/``end``; old spellings keep working as legacy aliases [see `PR #2367 `_] diff --git a/flexmeasures/cli/tests/test_utils.py b/flexmeasures/cli/tests/test_utils.py index 485f6cbbf1..c55745bc4c 100644 --- a/flexmeasures/cli/tests/test_utils.py +++ b/flexmeasures/cli/tests/test_utils.py @@ -178,6 +178,28 @@ def cmd(**kwargs): assert captured["model_params"] == {"max_depth": 6} +@pytest.mark.parametrize("bad_value", ["[1, 2]", "5", '"a string"']) +def test_add_cli_options_from_schema_rejects_non_mapping_dict_fields(bad_value): + """A parsable but non-mapping argument must be rejected by Click itself. + + Such a value would otherwise reach Marshmallow, which reports the unhelpful "Not a valid mapping type". + """ + from flexmeasures.cli.utils import add_cli_options_from_schema + from flexmeasures.data.schemas.forecasting.pipeline import ( + TrainPredictPipelineConfigSchema, + ) + + @click.command() + @add_cli_options_from_schema(TrainPredictPipelineConfigSchema()) + def cmd(**kwargs): + pass + + result = CliRunner().invoke(cmd, ["--model-params", bad_value]) + assert result.exit_code == 2, result.output + assert "Expected a mapping" in result.output + assert "Not a valid mapping type" not in result.output + + @pytest.mark.xfail( strict=True, raises=RuntimeError, diff --git a/flexmeasures/cli/utils.py b/flexmeasures/cli/utils.py index f4570a20fa..2c79baa06a 100644 --- a/flexmeasures/cli/utils.py +++ b/flexmeasures/cli/utils.py @@ -419,16 +419,25 @@ def convert(self, value, param, ctx): if isinstance(value, dict): return value try: - return json.loads(value) + parsed = json.loads(value) except json.JSONDecodeError: try: - return ast.literal_eval(value) + parsed = ast.literal_eval(value) except (ValueError, SyntaxError): self.fail( f"Cannot parse as a JSON object or Python-literal dict: {value!r}", param, ctx, ) + # A parsable non-object (e.g. a list or a number) would otherwise travel on, + # only to be rejected further downstream by Marshmallow with "Not a valid mapping type". + if not isinstance(parsed, dict): + self.fail( + f'Expected a mapping such as \'{{"key": "value"}}\', but got {type(parsed).__name__}: {value!r}', + param, + ctx, + ) + return parsed class JSONOrFile(click.ParamType):