Skip to content

Add localization columns to obs and responses endpoint#12730

Merged
SAKavli merged 2 commits intoequinor:mainfrom
SAKavli:add-localization-columns-to-obs-and-responses-endpoint
Jan 27, 2026
Merged

Add localization columns to obs and responses endpoint#12730
SAKavli merged 2 commits intoequinor:mainfrom
SAKavli:add-localization-columns-to-obs-and-responses-endpoint

Conversation

@SAKavli
Copy link
Contributor

@SAKavli SAKavli commented Jan 26, 2026

Issue
Resolves #12631

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

RFT observations have "north" and "east" as primary keys. Dropping
these before merging dataframes causes Ert to crash.

This will prevent dropping keys which overlap localization keys.
@SAKavli SAKavli force-pushed the add-localization-columns-to-obs-and-responses-endpoint branch 5 times, most recently from a0264eb to b4529d7 Compare January 27, 2026 07:26
@SAKavli SAKavli added the release-notes:bug-fix Automatically categorise as bug fix in release notes label Jan 27, 2026
@SAKavli
Copy link
Contributor Author

SAKavli commented Jan 27, 2026

Does this call for another test, or should the edited tests be coverage enough for this behavior?

@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.65%. Comparing base (cf69f75) to head (82eae6f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12730      +/-   ##
==========================================
- Coverage   90.65%   90.65%   -0.01%     
==========================================
  Files         432      432              
  Lines       30168    30175       +7     
==========================================
+ Hits        27349    27354       +5     
- Misses       2819     2821       +2     
Flag Coverage Δ
cli-tests 37.19% <100.00%> (-0.06%) ⬇️
gui-tests 68.56% <100.00%> (-0.02%) ⬇️
performance-and-unit-tests 77.04% <100.00%> (-0.01%) ⬇️
test 37.46% <0.00%> (-0.05%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SAKavli SAKavli force-pushed the add-localization-columns-to-obs-and-responses-endpoint branch from b4529d7 to 49eaeae Compare January 27, 2026 08:03
@SAKavli SAKavli marked this pull request as ready for review January 27, 2026 08:32
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 27, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing SAKavli:add-localization-columns-to-obs-and-responses-endpoint (82eae6f) with main (988451d)

Summary

✅ 23 untouched benchmarks

Copy link
Contributor

@eivindjahren eivindjahren left a comment

Choose a reason for hiding this comment

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

👍

@xjules
Copy link
Contributor

xjules commented Jan 27, 2026

One comment. Have you double checked whether the observations are actually in the field?
Update: as discussed you set gen_obs to null and summary_obs too.

@xjules
Copy link
Contributor

xjules commented Jan 27, 2026

If it is a bug-fix maybe it should contain the backport label to current testing?

@SAKavli SAKavli force-pushed the add-localization-columns-to-obs-and-responses-endpoint branch from 49eaeae to 108bd0a Compare January 27, 2026 09:40
@SAKavli SAKavli force-pushed the add-localization-columns-to-obs-and-responses-endpoint branch from 108bd0a to 82eae6f Compare January 27, 2026 09:41
@SAKavli
Copy link
Contributor Author

SAKavli commented Jan 27, 2026

Thanks for the feedback, Julius!

We decided to leave all test data to None for localization columns, as these values should make sense with the dimensions of the respective field. @xjules will test this in another PR and find some appropriate values for localization.

This should definitely be backported to February release - thanks!

@SAKavli SAKavli merged commit 1d70073 into equinor:main Jan 27, 2026
36 checks passed
@scout-team-app
Copy link

Successfully created backport PR for version-19.0:

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

Labels

backport version-19.0 release-notes:bug-fix Automatically categorise as bug fix in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Observation positional data filtered out when combining with responses

4 participants