fmuobs Add support for ERT observation with localization data#860
fmuobs Add support for ERT observation with localization data#860alifbe wants to merge 1 commit intoequinor:mainfrom
fmuobs Add support for ERT observation with localization data#860Conversation
alifbe
force-pushed
the
fmuobs-localization
branch
from
6daeeb5 to
19f3d1e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #860 +/- ##
==========================================
+ Coverage 83.52% 84.43% +0.90%
==========================================
Files 49 49
Lines 7279 7292 +13
==========================================
+ Hits 6080 6157 +77
+ Misses 1199 1135 -64 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds parsing support for extended ERT observation files that include LOCALIZATION { ... } sub-blocks, ensuring these files can be read by fmuobs without failing.
Changes:
- Add testdata ERT
.obsfile containingSUMMARY_OBSERVATIONentries withLOCALIZATIONblocks and anRFT_OBSERVATIONentry. - Extend the ERT obs flattener to skip unlabeled subunit blocks (to avoid failing on
LOCALIZATION). - Update/extend tests to include the new file and validate that
LOCALIZATIONis ignored while parsing remains correct.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/testdata_fmuobs/drogon_wbhp_rft_wct_gor_tracer_plt_local.obs | Adds an extended ERT obs example including LOCALIZATION blocks (and RFT CSV reference) for regression coverage. |
| tests/test_fmuobs.py | Includes the new .obs in parsing/roundtrip tests and adds a dedicated regression test for ignoring LOCALIZATION. |
| src/subscript/fmuobs/parsers.py | Changes flatten_observation_unit() to skip single-word subunit keys (e.g. LOCALIZATION) instead of raising. |
| src/subscript/eclcompress/eclcompress.py | Adjusts a comment (but currently risks reintroducing a Ruff line-length failure). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alifbe
force-pushed
the
fmuobs-localization
branch
from
19f3d1e to
ce01fce
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| for subunit in subunit_keys: | ||
| if len(subunit.split()) < 2: | ||
| if subunit.upper() in ("LOCALIZATION"): |
There was a problem hiding this comment.
subunit.upper() in ("LOCALIZATION") is using a string on the right-hand side, so it becomes a substring check (e.g., "LOC", "L" would also match) rather than an exact match. This can cause unrelated unlabeled subunit blocks to be silently ignored. Use an equality check (== "LOCALIZATION") or a 1-tuple (in ("LOCALIZATION",)) instead.
| if subunit.upper() in ("LOCALIZATION"): | |
| if subunit.upper() == "LOCALIZATION": |
Closes #851