Add options for statistics to sidebar - #14040
Merged
Merged
Conversation
MagnusSletten
force-pushed
the
stats-addition
branch
from
July 29, 2026 10:04
f3d1ba6 to
e788243
Compare
Collaborator
|
Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#70 |
ertomatic
pushed a commit
to equinor/ert-testdata
that referenced
this pull request
Jul 29, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14040 +/- ##
==========================================
- Coverage 91.83% 91.82% -0.02%
==========================================
Files 480 482 +2
Lines 33403 33444 +41
==========================================
+ Hits 30676 30709 +33
- Misses 2727 2735 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves a subset of plot “statistics customization” into the plot window sidebar by introducing explicit toggles (statistics visibility, area-vs-line bands, std-dev multiplier, and distribution connection lines), while removing the legacy plot customization dialog + its undo/redo config-history machinery.
Changes:
- Add new sidebar control widgets for statistics and distribution options, and apply them when building
PlotConfigper update. - Replace plot customizer/config-history usage with lightweight in-memory per-key title/x-label/y-label dictionaries.
- Remove the old customization dialog implementation and related tests, and update imports to use the new plot-controls
ColorBox.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ert/unit_tests/gui/tools/plot/test_plot_window.py | Update tests to reflect per-key title/axis-label storage (no PlotCustomizer). |
| tests/ert/unit_tests/gui/plottery/test_plot_config_history.py | Remove tests for deleted PlotConfigHistory. |
| tests/ert/ui_tests/gui/test_plot_customization.py | Remove UI test tied to the deleted customization dialog. |
| src/ert/gui/plotting/widgets/plot_widget.py | Remove customization-trigger plumbing from the plot widget/toolbar. |
| src/ert/gui/plotting/widgets/plot_controls/statistics_options.py | New sidebar widget for toggling statistics lines/bands + std-dev factor. |
| src/ert/gui/plotting/widgets/plot_controls/observation_color.py | Switch ColorBox import to new plot-controls module. |
| src/ert/gui/plotting/widgets/plot_controls/distribution_options.py | New sidebar widget for distribution connection lines toggle. |
| src/ert/gui/plotting/widgets/plot_controls/custom_palette_dialog.py | Switch ColorBox import to new plot-controls module. |
| src/ert/gui/plotting/widgets/plot_controls/color_chooser.py | New ColorBox implementation used by sidebar palette/observation color UI. |
| src/ert/gui/plotting/widgets/plot_controls/init.py | Export new StatisticsOptions and DistributionOptions. |
| src/ert/gui/plotting/widgets/copy_style_to_dialog.py | Delete dialog tied to legacy customization/copy workflow. |
| src/ert/gui/plotting/widgets/init.py | Stop exporting removed CopyStyleToDialog. |
| src/ert/gui/plotting/utils/qt_creator.py | Add collapsible group box + spinbox/labeled-row helpers for sidebar UI. |
| src/ert/gui/plotting/utils/plot_config_history.py | Delete PlotConfigHistory implementation. |
| src/ert/gui/plotting/utils/init.py | Stop exporting removed PlotConfigHistory. |
| src/ert/gui/plotting/plot_window.py | Wire new sidebar options into PlotConfig creation and remove PlotCustomizer usage. |
| src/ert/gui/plotting/customization_dialog/style_customization_view.py | Delete legacy customization dialog view. |
| src/ert/gui/plotting/customization_dialog/style_chooser.py | Delete legacy style chooser. |
| src/ert/gui/plotting/customization_dialog/statistics_customization_view.py | Delete legacy statistics customization view. |
| src/ert/gui/plotting/customization_dialog/customize_plot_dialog.py | Delete legacy customization dialog + PlotCustomizer. |
| src/ert/gui/plotting/customization_dialog/customization_view.py | Delete legacy customization dialog base view code. |
| src/ert/gui/plotting/customization_dialog/init.py | Remove customization_dialog package exports. |
MagnusSletten
force-pushed
the
stats-addition
branch
from
July 29, 2026 12:47
e788243 to
a512053
Compare
MagnusSletten
force-pushed
the
stats-addition
branch
from
July 29, 2026 12:51
a512053 to
ef4b430
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/ert/gui/plotting/widgets/plot_controls/statistics_options.py:92
- StatisticsOptions.apply_to introduces new behavior (mapping checkboxes to PlotConfig statistics styles, area-vs-lines behavior, and std dev multiplier), but there are currently no unit tests asserting that these controls actually update PlotConfig as intended. Adding focused tests for apply_to (e.g. enabling/disabling a statistic changes its line_style visibility, the Area toggle switches band line_style between "--" and "#", and the spinbox sets the standard deviation factor) would help prevent regressions.
def apply_to(self, plot_config: PlotConfig) -> None:
"""Enable or disable each statistic according to its checkbox."""
plot_config.set_standard_deviation_factor(self._std_dev_factor.value())
fill_area = self._area_toggle.isChecked()
for statistic, checkbox in self._toggles.items():
style = plot_config.get_statistics_style(statistic)
if not checkbox.isChecked():
style.line_style = ""
style.marker = ""
elif statistic in _BAND_STATISTICS:
style.line_style = _AREA_LINE_STYLE if fill_area else _BAND_LINE_STYLE
style.marker = ""
elif not style.is_visible():
style.line_style = _LINE_STYLE_WHEN_ENABLED[statistic]
style.marker = ""
plot_config.set_statistics_style(statistic, style)
src/ert/gui/plotting/widgets/plot_controls/distribution_options.py:39
- DistributionOptions.apply_to adds new plot behavior (toggling PlotConfig.set_distribution_line_enabled), but there is no corresponding unit test coverage for this new control. Please add a small test that toggling the checkbox results in the expected PlotConfig.is_distribution_line_enabled() value after apply_to.
def apply_to(self, plot_config: PlotConfig) -> None:
plot_config.set_distribution_line_enabled(
self._connection_lines_toggle.isChecked()
)
src/ert/gui/plotting/widgets/plot_controls/statistics_options.py:30
- The default toggle states in _STATISTICS hard-code which statistics are shown (e.g. std defaults to False, p10-p90 defaults to True). This duplicates/overrides PlotConfigFactory’s dimensionality-based defaults (e.g. PlotConfigFactory enables std for 1D keys and p10-p90 for 2D keys), which can change the default plot output for 1D data and makes the factory defaults effectively irrelevant once these options are applied. Consider making a single source of truth for the defaults (either align these initial_checked values with PlotConfigFactory, or move the default-selection logic into apply_to based on the incoming PlotConfig/key dimensionality).
# (statistic, label, tooltip, checked by default)
_STATISTICS = [
("mean", "Mean", "Show or hide the mean", True),
("p50", "P50", "Show or hide the P50 line", False),
("std", "Std dev", "Show or hide the standard deviation band", False),
("min-max", "Min/Max", "Show or hide the min/max band", False),
("p10-p90", "P10-P90", "Show or hide the P10-P90 band", True),
("p33-p67", "P33-P67", "Show or hide the P33-P67 band", False),
]
MagnusSletten
force-pushed
the
stats-addition
branch
from
July 29, 2026 13:04
ef4b430 to
6bd9430
Compare
MagnusSletten
force-pushed
the
stats-addition
branch
6 times, most recently
from
July 30, 2026 11:32
0db566b to
7e8f36f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/ert/gui/plotting/utils/qt_creator.py:118
- Same issue as for checkboxes: QSpinBox.valueChanged emits the new integer value, and when the connection point is PlotWindow.update_plot this value is treated as the layer index. Changing the std-dev multiplier can therefore jump layers unexpectedly. Connect via a wrapper that discards the emitted value.
spinbox.valueChanged.connect(connection_point)
src/ert/gui/plotting/utils/qt_creator.py:98
- These sidebar controls connect Qt signals that emit values (e.g. 0/2 for checkboxes) directly to PlotWindow.update_plot. Since update_plot interprets its optional positional argument as the plot layer index, toggling a checkbox can unintentionally change the active layer (and fetch/std-dev data for the wrong layer) instead of just refreshing the plot. Wrap the connection point so the signal arguments are ignored.
This issue also appears on line 118 of the same file.
checkbox.stateChanged.connect(connection_point)
MagnusSletten
force-pushed
the
stats-addition
branch
3 times, most recently
from
July 31, 2026 07:00
2ab52bf to
cf5789f
Compare
MagnusSletten
marked this pull request as ready for review
July 31, 2026 07:01
MagnusSletten
force-pushed
the
stats-addition
branch
from
July 31, 2026 08:03
cf5789f to
5f340d7
Compare
eilskra
reviewed
Jul 31, 2026
| @@ -0,0 +1,26 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| STATISTIC_LABELS = { | |||
Contributor
There was a problem hiding this comment.
Not important, but could this be simplified along the lines of outline below?
STATISTICS = {
"mean": ("Mean", "-")
"p50": ("P50", "--")
"std": ("Std dev", "--")
....
Edit: highlighted that this is a question
Contributor
Author
There was a problem hiding this comment.
Named tuple looks quite nice here. Whole file becomes:
class StatisticStyle(NamedTuple):
label: str
line_style: str
is_band: bool
STATISTICS = {
"mean": StatisticStyle("Mean", "-", is_band=False),
"p50": StatisticStyle("P50", "--", is_band=False),
"std": StatisticStyle("Std dev", ":", is_band=True),
"min-max": StatisticStyle("Min/Max", ":", is_band=True),
"p10-p90": StatisticStyle("P10-P90", "--", is_band=True),
"p33-p67": StatisticStyle("P33-P67", "-.", is_band=True),
}
BAND_AREA_STYLE = "#"
DEFAULT_ENABLED_STATISTICS = {"mean", "p10-p90"}
Comment on lines
+18
to
+24
| "mean": "-", | ||
| "p50": "--", | ||
| "std": "--", | ||
| "min-max": "--", | ||
| "p10-p90": "--", | ||
| "p33-p67": "--", | ||
| } |
Contributor
There was a problem hiding this comment.
Should we include more styles? e.g .- etc
Contributor
Author
There was a problem hiding this comment.
Different defaults here is an improvement, I agree. For instance
STATISTICS_LINE_STYLES = {
"mean": "-",
"p50": "--",
"std": ":",
"min-max": ":",
"p10-p90": "--",
"p33-p67": "-.",
}
MagnusSletten
force-pushed
the
stats-addition
branch
2 times, most recently
from
July 31, 2026 10:09
19ad595 to
97c80da
Compare
eilskra
approved these changes
Jul 31, 2026
eilskra
left a comment
eilskra
left a comment
Contributor
There was a problem hiding this comment.
One curiosity and one potential simplification suggestion, they do not matter, so no need to implement. LGTM 👍
| "p33-p67": PlotStyle("P33-P67", line_style=""), | ||
| "std": PlotStyle("Std dev", line_style=""), | ||
| statistic: PlotStyle(style.label, line_style="") | ||
| for statistic, style in STATISTICS.items() |
Contributor
There was a problem hiding this comment.
is this needed? could we not in line 85 just do:
for statistic, style, _ in n STATISTICS.items():
Contributor
Author
There was a problem hiding this comment.
It's shorthand for:
self._statistics_style = {}
for statistic, style in STATISTICS.items():
self._statistics_style[statistic] = PlotStyle(
style.label,
line_style="",
)
Contributor
There was a problem hiding this comment.
I meant more, why do we need self._statistics_style = {}, could we not just use STATISTICS.items()?
Contributor
Author
There was a problem hiding this comment.
statistics_style are the mutable style elements in the config file which is changed by the options.
STATISTICS.items() are just the defaults styles and the rules.
At least the line styles are changed with the band option. On/off switches as well.
eilskra
reviewed
Jul 31, 2026
| elif fill_bands and STATISTICS[statistic].is_band: | ||
| style.line_style = BAND_AREA_STYLE | ||
| else: | ||
| style.line_style = STATISTICS[statistic].line_style |
Contributor
There was a problem hiding this comment.
This was the section I meant why we couldn't use STATISTICS.items() directly, but I see now it overwrites
Statistics styles were derived from the plot dimensionality, but they are only read by the statistics plotter, which is always dimensionality 2. The remaining branches were dead code. Replace the coupling with an explicit selection of which statistics to draw. This also simplifies statistics styling in general: each statistic has a fixed style and is only turned on or off. The default selection reproduces the previous appearance.
The statistics customization tab disappeared with the plot customization dialog, leaving no way to choose which statistics to show. Reintroduce the controls in the plot sidebar, letting the user select statistics and whether bands are filled.
MagnusSletten
force-pushed
the
stats-addition
branch
from
July 31, 2026 11:32
97c80da to
fb84e73
Compare
MagnusSletten
added
the
improvement
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
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.
Issue
Resolves #14030
Approach
Will add a simplified version of statistics controls to the sidebar, with toggles for different lines, e.g p50/p99.
It also turns out that some of the complications around style defaults and dimensionality was not warranted because 1 dimensional style defaults were in effect never used in the repository. This is simplified in the PR by removing the dimensionality specific styling code.
Visual changes
Added collapsible statistics panel:
git rebase -i main --exec 'just rapid-tests')When applicable
merge screenshot-PR in ert-testdata before merging this PR.