Skip to content

Add RFT QC widget status column test - #14355

Open
HakonSohoel wants to merge 1 commit into
equinor:mainfrom
HakonSohoel:add-rft-status-column-test
Open

Add RFT QC widget status column test#14355
HakonSohoel wants to merge 1 commit into
equinor:mainfrom
HakonSohoel:add-rft-status-column-test

Conversation

@HakonSohoel

@HakonSohoel HakonSohoel commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #14330

  • 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')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@HakonSohoel HakonSohoel added the release-notes:skip If there should be no mention of this in release notes label Sep 3, 2026
@HakonSohoel

Copy link
Copy Markdown
Contributor Author

Created the test to see if the rft qc widget was affected by a polars bug (#14277 (comment)). It turned out it was not affected, but this test still verifies that the response status column is added as expected both with and without response approximation, so it is worth keeping.

@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.55%. Comparing base (4e36dd4) to head (1219c31).
⚠️ Report is 8 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (4e36dd4) and HEAD (1219c31). Click for more details.

HEAD has 13 uploads less than BASE
Flag BASE (4e36dd4) HEAD (1219c31)
gui-tests 4 2
test 4 2
fuzz 3 2
performance-and-unit-tests 4 0
cli-tests 4 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #14355       +/-   ##
===========================================
- Coverage   91.75%   70.55%   -21.21%     
===========================================
  Files         485      497       +12     
  Lines       34708    36062     +1354     
===========================================
- Hits        31848    25444     -6404     
- Misses       2860    10618     +7758     
Flag Coverage Δ
cli-tests ?
fuzz 43.85% <ø> (-0.04%) ⬇️
gui-tests 58.04% <ø> (-0.14%) ⬇️
performance-and-unit-tests ?
test 45.27% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 266 files with indirect coverage changes

@codspeed-hq

codspeed-hq Bot commented Sep 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing HakonSohoel:add-rft-status-column-test (1219c31) with main (112f2c8)

Open in CodSpeed

@achaikou achaikou 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.

⭐ 👍


stored_observations = pl.DataFrame(
observations,
schema={

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.

Just a note:

To keep schemas consistent with possible future changes might be useful to use
defaults_generator's create_rft_observation + create_observation_dataframe's _handle_rft_observation (or just add new create_rft_observation_dataframe function to defaults_generator)

Also might be nice adding create_rft_response consistent with default rft observations (in the lines of create_seismic_response) to use standard response schema and get rid of explicitly defining uninteresting defaults like well-date-pressure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:skip If there should be no mention of this in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test case to rft qc tool

3 participants