Skip to content

Update PlotApi to use StorageApi for fetching data - #14270

Open
eqbech wants to merge 1 commit into
mainfrom
replace-plot-api-with-storage-api
Open

Update PlotApi to use StorageApi for fetching data#14270
eqbech wants to merge 1 commit into
mainfrom
replace-plot-api-with-storage-api

Conversation

@eqbech

@eqbech eqbech commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #14244

Approach
Short description of the approach

(Screenshot of new behavior in GUI if applicable)

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

Stack created with GitHub Stacks CLIGive Feedback 💬

@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 645fb56 to 0c237f4 Compare August 25, 2026 07:48
@codspeed-hq

codspeed-hq Bot commented Aug 25, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing replace-plot-api-with-storage-api (b8a5ec6) with main (23d74ff)

Open in CodSpeed

@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 0c237f4 to cd7414d Compare August 25, 2026 12:59
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from cd7414d to 650fa16 Compare August 26, 2026 07:07
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 650fa16 to 62662d2 Compare August 26, 2026 10:00
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 62662d2 to 0229be1 Compare August 27, 2026 06:01
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 0229be1 to 1c33eb5 Compare August 27, 2026 06:03
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#96

@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshot tests now pass. The baseline update PR equinor/ert-testdata#96 has been closed.

@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#97

@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 1c33eb5 to 846cdb5 Compare August 27, 2026 06:16
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Aug 27, 2026
@codecov-commenter

codecov-commenter commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.97297% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.14%. Comparing base (23d74ff) to head (b8a5ec6).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/ert/gui/plotting/plot_api.py 70.76% 38 Missing ⚠️
src/ert/gui/plotting/plot_window.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14270      +/-   ##
==========================================
+ Coverage   91.11%   91.14%   +0.03%     
==========================================
  Files         498      498              
  Lines       36224    36176      -48     
==========================================
- Hits        33004    32974      -30     
+ Misses       3220     3202      -18     
Flag Coverage Δ
cli-tests 35.56% <5.40%> (+0.05%) ⬆️
fuzz 43.70% <10.13%> (+<0.01%) ⬆️
gui-tests 58.36% <64.86%> (+0.11%) ⬆️
performance-and-unit-tests 81.11% <68.91%> (+0.04%) ⬆️
test 45.26% <10.13%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ert/services/ert_client.py 78.83% <100.00%> (+3.64%) ⬆️
src/ert/services/shared_client.py 68.68% <100.00%> (+13.84%) ⬆️
src/ert/gui/plotting/plot_window.py 78.84% <33.33%> (-0.05%) ⬇️
src/ert/gui/plotting/plot_api.py 80.40% <70.76%> (+0.20%) ⬆️

... and 1 file with indirect coverage changes

@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 846cdb5 to 31a602a Compare August 27, 2026 08:45
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshot tests now pass. The baseline update PR equinor/ert-testdata#97 has been closed.

@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 31a602a to 453e49c Compare August 27, 2026 11:10
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 453e49c to d340629 Compare August 27, 2026 12:40
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from d340629 to 6560040 Compare August 27, 2026 12:44
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from d473c67 to c6760bc Compare August 31, 2026 14:03
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Aug 31, 2026
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshot tests now pass. The baseline update PR equinor/ert-testdata#103 has been closed.

@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#108

@andreas-el andreas-el added this to SCOUT Sep 1, 2026
@andreas-el andreas-el moved this to Ready for Review in SCOUT Sep 1, 2026
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 8ae9517 to 3973c8b Compare September 1, 2026 10:46
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshot tests now pass. The baseline update PR equinor/ert-testdata#108 has been closed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refactors the GUI plotting layer (PlotApi/PlotWindow) to fetch data via the shared storage-server client (ErtClient/SharedClient) instead of using ad-hoc HTTP client creation, aligning with issue #14244’s goal of a single reusable client for the shared server.

Changes:

  • Update PlotApi to use ErtClient for experiments/ensembles/parameters/responses/gradients/observations, and move caching to per-PlotApi instance caches.
  • Update PlotWindow and related tests to call instance methods (api.data_for_parameter(...), etc.) instead of static cached functions requiring ens_path.
  • Add SharedClient.close_client() and ensure GUI UI tests reset the process-wide shared client between tests.
File summaries
File Description
tests/ert/unit_tests/gui/tools/plot/test_plot_window.py Adjust PlotWindow tests to mock PlotApi instance methods and updated data_for_parameter signature.
tests/ert/unit_tests/gui/tools/plot/test_plot_api.py Adapt PlotApi tests to new client plumbing by monkeypatching ErtClient.get_client and escaping behavior.
tests/ert/unit_tests/gui/tools/plot/conftest.py Provide PlotApi fixture using an injected ErtClient wrapper over a mock request client.
tests/ert/ui_tests/gui/conftest.py Add an autouse teardown fixture to close/reset the process-wide SharedClient between GUI UI tests.
tests/ert/performance_tests/test_dark_storage_performance.py Update performance test client wiring to match ErtClient-based PlotApi behavior.
src/ert/services/shared_client.py Add SharedClient.close_client() for explicit lifecycle reset of the singleton client.
src/ert/services/ert_client.py Switch parquet decoding/caching to pandas DataFrames and expose DataFrame-returning endpoints.
src/ert/gui/plotting/plot_window.py Use PlotApi instance methods for data fetching (no longer passes ens_path into static functions).
src/ert/gui/plotting/plot_api.py Replace direct HTTP calls with ErtClient calls and implement per-instance caching wrappers.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ert/services/ert_client.py
Comment thread src/ert/services/ert_client.py
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch from 3973c8b to 5496190 Compare September 1, 2026 12:08
Comment thread src/ert/gui/plotting/plot_api.py
Comment thread src/ert/gui/plotting/plot_api.py
Comment thread src/ert/gui/plotting/plot_api.py
Comment thread tests/ert/unit_tests/gui/tools/plot/test_plot_api.py
Comment thread src/ert/gui/plotting/plot_api.py Outdated
@berland

berland commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

git commit body says "format". That is probably not intentional and can be improved.

Comment thread src/ert/gui/plotting/plot_api.py
@eqbech
eqbech force-pushed the replace-plot-api-with-storage-api branch 2 times, most recently from 8da2d38 to 3bf2e5e Compare September 4, 2026 08:03

@berland berland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get it in!

@github-project-automation github-project-automation Bot moved this from Ready for Review to Reviewed in SCOUT Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:improvement Automatically categorise as improvement in release notes

Projects

Status: Reviewed

Development

Successfully merging this pull request may close these issues.

Create a common ert/everest client

7 participants