feat: add plot descriptions to sensors_to_show - #2490
Open
VITA2aishu wants to merge 3 commits into
Open
VITA2aishu wants to merge 3 commits into
VITA2aishu wants to merge 3 commits into
Conversation
Documentation build overview
7 files changed ·
|
Author
|
Hi, I pushed a fix for the schema lint error. Lint, formatting, type checks, and OpenAPI generation now pass locally. The schema tests report 37 passed and 2 PostgreSQL setup errors. UI and full-suite validation remain incomplete because PostgreSQL is unavailable locally. Could you please authorize the PR workflows so testing can run with PostgreSQL? I’m keeping the PR in draft until validation is complete. Thank you! |
Member
|
I authorized the PR workflows. |
…trains on (FlexMeasures#2482) * forecasting: go back no further than either training bound asks for Context: - train-start and train-period each say how far back to train, and a config may state both. - Since FlexMeasures#2178 a stated train-start won outright, so a stated train-period could no longer narrow it, and a config asking for 30 days of history could train on a year without saying so. - That FlexMeasures#2178 change was right about the default: train-period carries a load default of 30 days, which used to narrow a stated train-start silently, which is what issue FlexMeasures#2186 was about. - The two cases were indistinguishable, because by the time the config is loaded a stated period looks exactly like a defaulted one. Change: - Remember at load time whether train-period was asked for, rather than defaulted to. - Let a stated train-period take part in the decision again, so stating both goes back no further than either asks for, while a defaulted one still stays out of the way of a stated train-start. - Say at debug level which bound decided, and how far back the window reaches. The resulting window is unchanged for every config that does not state both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * tests: cover which training bound decides the window Context: - The window a config resolves to had no test of its own, so the change of precedence in FlexMeasures#2178 could pass unnoticed. Change: - Cover each shape a config can take: neither bound stated, either one stated, and both stated with either one asking for less. - Assert that a stated train-period is told apart from the defaulted one, since that is what lets the two cases differ. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * docs: describe the training window the CLI actually derives Context: - The help for `flexmeasures add forecasts` still described a 30-day training window as the rule, which stopped holding once a stated train-start took over. Change: - Say that a stated train-start decides instead, capped to max-training-period, and what stating both amounts to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * docs: changelog entry for the training window bounds Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * forecasting: keep a defaulted train-period defaulted when a config is stored Context: - Forecaster configs are stored by dumping them, and read back by loading that dump, see DataGenerator.data_source. - A dumped config always carries a train-period, defaulted or not, so a defaulted one read back as though it had been asked for, and would have started narrowing the stated train-start. - That would have brought back the very behaviour issue FlexMeasures#2186 was about, for every stored forecaster config. Change: - Make the answer part of the config, so that it is stored alongside the period it describes. - Keep the answer a config already carries, and settle it from the config as given only when it carries none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * tests: cover storing a config and reading it back, and asking for no period Context: - A stored config that read back as though its defaulted period had been asked for would silently narrow the training window, which is what the round trip now pins down. Change: - Assert that both a defaulted and an asked-for train-period survive being stored and read back as what they were. - Cover asking for no period at all, which leaves only the outer bound. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * docs: break the forecasting help and docstrings after punctuation Context: - Several lines in the `add forecasts` help and in the training window docstring wrapped mid-phrase, which the repo's docstring rule does not allow. Change: - End each physical line at a comma or a period, keeping the wording as it was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * forecasting: leave a defaulted train-period out of the stored config Context: - Whether the period was asked for was kept in a setting of its own, which put bookkeeping in the config that hosts would see and could set. - The config as stored can say the same thing on its own: a period that was asked for is written out, and one that was only defaulted to is not. Change: - Drop the setting, and leave a defaulted train-period out of the config as dumped. - Reading such a config back finds no period, which is what it was, so storing and reading back stays faithful without a field to carry the answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * tests: assert the config keeps no bookkeeping of its own Context: - Whether the period was asked for now rides on train-period being in the stored config at all, so nothing about it should show up as a setting hosts can see or set. Change: - Assert that a stored config carries no marker of its own, leaves out a period nobody asked for, and keeps one that was asked for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * forecasting: make train-period the one way to limit how much history is used Context: - train-period and max-training-period both said how far back training may reach, differing only in their default and in when they applied, which left two ways to say one thing. - Telling a stated period from a defaulted one, so that a default would not narrow a stated train-start, was what made the config as stored differ from the config as written, and needed bookkeeping to survive being stored and read back. Change: - Let train-start say where training may begin and train-period say how much history to use, and take whichever of the two asks for less data. Both are limits, so the window is what they both allow. - Fold max-training-period into train-period, which now says the same thing, and keep the old name working as a deprecated alias, on the command line and in configs written before the two were merged. A config carrying both is asking twice, so the shorter of the two decides. - Drop the bookkeeping that told a stated period from a defaulted one, along with the capping that only existed because there were two limits to reconcile. A config that names only a start now trains on 30 days rather than back to the start, which is what it did before the two settings drifted apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * tests: cover the one training limit and its deprecated alias Context: - The window a config resolves to had no test of its own, so a change of precedence could pass unnoticed. Change: - Cover each shape a config can take: neither limit named, either one named, and both named with either one asking for less. - Cover the deprecated max-training-period, on its own and alongside train-period, and that it is no longer written back out. - Drop the tests for the capping and for the bookkeeping that told a stated period from a defaulted one, neither of which the config does any more. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * docs: describe the two training limits and the deprecated alias Change: - Say in the `add forecasts` help that train-start says where training may begin and train-period says how much history to use, and that max-training-period is now a deprecated alias of the latter. - Record both in the changelog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * forecasting: say why a training period in years or months does not work Context: - A period given in years or months, such as P1Y, parses to a Duration rather than a timedelta, and the two cannot be compared. - The check that says so ran after the one comparing the period to two days, so the comparison raised a TypeError first and the reason never reached the user. - P1Y was the documented example of the setting this one absorbs, so it was a likely thing to write. Change: - Say it before anything measures the period. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * tests: cover a training period given in years or months Change: - Assert that a period in years or months is reported as unsupported, under either name, rather than failing to compare. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * forecasting: report a bad training limit under either name Context: - Reading the deprecated name as the one that remains compared the two to keep the shorter, which failed in two ways. - A limit in years or months parses to a Duration, which cannot be compared to a timedelta, so a config naming one alongside a plain one raised a TypeError. - A limit that could not be parsed at all was passed over in favour of the other, so a config could name nonsense and be accepted. Change: - Work out which of the two is a length that can be measured, before comparing them. - Hand a limit that is not to the field, which says what is wrong with it, rather than going with the other one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> * tests: cover a bad training limit under either name Change: - Assert that a limit which cannot be measured is reported, whichever of the two names it was given under, rather than passed over for the other one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HeRZd2nDDShZTE8ZmbERzn Signed-off-by: F.N. Claessen <claessen@seita.nl> --------- Signed-off-by: F.N. Claessen <claessen@seita.nl> Signed-off-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Aisvarya Sampath Kumar <aisvarya20@gmail.com>
Signed-off-by: Aisvarya Sampath Kumar <aisvarya20@gmail.com>
Build the shared plot metadata once instead of using a nested factory. Preserve optional descriptions and existing schema output. Assisted-by: OpenAI Codex Signed-off-by: Aisvarya Sampath Kumar <aisvarya20@gmail.com>
VITA2aishu
force-pushed
the
feature/1194-plot-description
branch
from
September 14, 2026 17:10
a543eb7 to
28b1904
Compare
Author
|
Thanks! I’ve now added the required DCO sign-offs, and the DCO check is passing. The updated commits triggered the workflows again. Could you please authorize them when you get a chance? Thank you! |
Member
|
Done! |
VITA2aishu
marked this pull request as ready for review
September 14, 2026 17:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds optional plot descriptions to sensors_to_show for #1194.
This updates the schema, graph editor/UI handling, ECharts rendering, documentation, changelog, and related tests while keeping existing configurations backward compatible when no description is provided.
The description is shown as small text below the subchart title, following the design discussion in the issue.
Testing notes:
Static typing checks passed.
Some UI tests could not complete locally because the test environment could not connect to the required local PostgreSQL service.
OpenAPI spec generation failed locally because WSL is not installed.
Related issue: #1194