Skip to content

fmuobs Fix starttime/startdate not being applied to compute_date_from days#853

Merged
alifbe merged 1 commit intoequinor:mainfrom
alifbe:fmuobs-fix-startdate
Mar 19, 2026
Merged

fmuobs Fix starttime/startdate not being applied to compute_date_from days#853
alifbe merged 1 commit intoequinor:mainfrom
alifbe:fmuobs-fix-startdate

Conversation

@alifbe
Copy link
Collaborator

@alifbe alifbe commented Mar 18, 2026

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes handling of ERT SUMMARY_OBSERVATION entries that only specify DAYS (no DATE) by ensuring --starttime/--startdate is actually applied when computing dates, and adds an integration test + testdata to cover the behavior.

Changes:

  • Add ERT .obs testdata containing summary observations with DAYS but no DATE.
  • Add an integration test asserting YAML export fails without --startdate and succeeds with it (and validates computed dates).
  • Pass starttime through to compute_date_from_days() in fmuobs() so computed dates are created.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/testdata_fmuobs/summary_obs_without_date.obs New test fixture with summary observations lacking DATE (using DAYS).
tests/test_fmuobs.py New integration test for --startdate behavior and date computation in YAML output.
src/subscript/fmuobs/fmuobs.py Fix: ensure starttime is provided to compute_date_from_days().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.55%. Comparing base (88003f2) to head (56e5c9f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #853      +/-   ##
==========================================
+ Coverage   83.52%   83.55%   +0.02%     
==========================================
  Files          49       49              
  Lines        7279     7279              
==========================================
+ Hits         6080     6082       +2     
+ Misses       1199     1197       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alifbe alifbe force-pushed the fmuobs-fix-startdate branch from 0ad8d25 to b0370ad Compare March 18, 2026 17:07
@alifbe alifbe requested a review from Copilot March 18, 2026 17:07
@alifbe alifbe changed the title fmuobs Fix bug summary observation without date fmuobs Fix starttime/startdate not being applied to compute_date_from days Mar 18, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes fmuobs handling of ERT SUMMARY_OBSERVATION entries that specify DAYS but omit DATE, by ensuring the CLI --starttime/--startdate argument is actually used when computing dates before YAML export.

Changes:

  • Pass starttime through to compute_date_from_days() in the fmuobs() conversion flow.
  • Add an integration test asserting failure without --startdate and correct computed dates with --startdate.
  • Add new ERT .obs testdata containing summary observations without explicit DATE.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/subscript/fmuobs/fmuobs.py Ensures starttime is applied when computing DATE from DAYS prior to validation/output.
tests/test_fmuobs.py Adds integration coverage for --startdate behavior and output date computation.
tests/testdata_fmuobs/summary_obs_without_date.obs Adds representative ERT observation input lacking DATE fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@alifbe alifbe force-pushed the fmuobs-fix-startdate branch 2 times, most recently from 86e1bad to d5193be Compare March 18, 2026 18:01
@alifbe alifbe requested a review from Copilot March 18, 2026 18:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes fmuobs so the --starttime/--startdate CLI argument is actually applied when converting DAYS-based observations into computed DATEs, and adds an integration test + testdata to prevent regressions.

Changes:

  • Pass starttime through to ertobs2df(...) consistently and into compute_date_from_days(...) so DAYS -> DATE conversion is applied.
  • Add a new integration test covering summary observations without explicit dates, validating both failure-without-startdate and correct computed dates with --startdate.
  • Add ERT .obs testdata containing summary observations with DAYS but no DATE.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/subscript/fmuobs/fmuobs.py Ensures starttime is used when computing dates from DAYS and when reparsing ERT observations with include dirs.
tests/test_fmuobs.py Adds an integration test verifying --startdate enables DAYS -> DATE conversion and validates output dates.
tests/testdata_fmuobs/summary_obs_without_date.obs New test input containing summary observations with DAYS and no explicit dates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@alifbe alifbe force-pushed the fmuobs-fix-startdate branch from d5193be to 56e5c9f Compare March 18, 2026 19:46
@alifbe alifbe requested review from berland, larsevj and rnyb March 18, 2026 19:47
Copy link
Collaborator

@rnyb rnyb left a comment

Choose a reason for hiding this comment

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

LGTM

@alifbe alifbe merged commit 6d79bc1 into equinor:main Mar 19, 2026
6 checks passed
@alifbe alifbe deleted the fmuobs-fix-startdate branch March 19, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants