Allow concat of history and summary obs#12674
Conversation
This applies to Summary and History observations - as these are strictly speaking the same type. As they are concatinated, we need their dataframe to be of same width.
This allows SummaryObservations and HistoryObservations to be concatinated to a single table - as this requires the same column-type.
8c88bd1 to
2b9b7a4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12674 +/- ##
==========================================
- Coverage 90.68% 90.64% -0.04%
==========================================
Files 429 430 +1
Lines 29808 29814 +6
==========================================
- Hits 27030 27026 -4
- Misses 2778 2788 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Pull request overview
This PR adds location-related fields (location_x, location_y, location_range) to observation dataframes to enable concatenation of history and summary observations. The changes include a storage migration (to version 22), updates to observation creation logic, and comprehensive test coverage.
Changes:
- Adds three location fields with Float32 dtype and None defaults to both history and summary observations
- Implements storage migration to22 for backward compatibility
- Updates observation creation to consistently include location fields
- Adds tests for migration and new functionality
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/ert/storage/migration/to22.py |
New migration adding location fields to existing summary observations |
src/ert/storage/local_storage.py |
Storage version bump to 22 and migration registration |
src/ert/config/_create_observation_dataframes.py |
Updated observation creation to include location fields |
tests/ert/unit_tests/storage/migration/test_to22.py |
Migration tests covering various scenarios |
tests/ert/unit_tests/config/test_summary_config.py |
Tests for location field handling |
| Snapshot files (3 files) | Updated snapshots with new location fields |
d65c0c2 to
e5d51b0
Compare
eivindjahren
left a comment
There was a problem hiding this comment.
Looks good! Difficult to be sure about these snapshots, but they only add location info so it is rather minor.
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin version-19.0
git worktree add -d .worktree/backport-12674-to-version-19.0 origin/version-19.0
cd .worktree/backport-12674-to-version-19.0
git switch --create backport-12674-to-version-19.0
git cherry-pick -x 7238d60a03af865889e22d8d61f62471e103a450 |
* Add default Null localization values This applies to Summary and History observations - as these are strictly speaking the same type. As they are concatinated, we need their dataframe to be of same width. * Consistently use pl.Float32 for localization This allows SummaryObservations and HistoryObservations to be concatinated to a single table - as this requires the same column-type. * Update snapshots * Create storage migration (cherry picked from commit 7238d60)
|
Successfully created backport PR for |
* Add default Null localization values This applies to Summary and History observations - as these are strictly speaking the same type. As they are concatinated, we need their dataframe to be of same width. * Consistently use pl.Float32 for localization This allows SummaryObservations and HistoryObservations to be concatinated to a single table - as this requires the same column-type. * Update snapshots * Create storage migration (cherry picked from commit 7238d60)
Issue
Resolves #12620
Will squash partially or completely