Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure SummaryConfig cannot be created with no keys #7150

Merged

Conversation

eivindjahren
Copy link
Contributor

@eivindjahren eivindjahren commented Feb 9, 2024

Resolves #7046

  • Ensures good behavior regarding setting ECLBASE but not using it.
  • Document behavior of existence check in local_ensemble
  • Updates and refactors stateful storage test to reflect this behavior

image

  • 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 tests pass locally (after every commit!)

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: Create Backport PR to latest release

@eivindjahren eivindjahren force-pushed the refactor_statefultest_to_allow_deletion branch from 0fb444f to c29b7b2 Compare February 15, 2024 21:50
@eivindjahren eivindjahren changed the title Refactor stateful test so adding deletion is easy Ensure empty parameter and empty responses are handled consistently in storage Feb 15, 2024
@eivindjahren eivindjahren force-pushed the refactor_statefultest_to_allow_deletion branch 3 times, most recently from 47a573d to 46953f1 Compare February 16, 2024 00:03
@eivindjahren eivindjahren force-pushed the refactor_statefultest_to_allow_deletion branch from 46953f1 to 6da9b91 Compare February 16, 2024 00:04
@eivindjahren eivindjahren changed the title Ensure empty parameter and empty responses are handled consistently in storage Ensure SummaryConfig cannot be created with no keys Feb 16, 2024
@eivindjahren eivindjahren force-pushed the refactor_statefultest_to_allow_deletion branch from 6da9b91 to a5b5745 Compare February 16, 2024 06:58
@eivindjahren eivindjahren self-assigned this Feb 16, 2024
@eivindjahren eivindjahren force-pushed the refactor_statefultest_to_allow_deletion branch 2 times, most recently from 0831be9 to 728aea3 Compare February 16, 2024 09:28
@eivindjahren eivindjahren marked this pull request as draft February 16, 2024 09:43
Copy link
Contributor

@frode-aarstad frode-aarstad left a comment

Choose a reason for hiding this comment

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

Looks good!

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (5344f25) 84.71% compared to head (0037825) 85.41%.
Report is 6 commits behind head on main.

Files Patch % Lines
src/ert/config/observations.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7150      +/-   ##
==========================================
+ Coverage   84.71%   85.41%   +0.69%     
==========================================
  Files         380      380              
  Lines       22633    22631       -2     
  Branches      931      936       +5     
==========================================
+ Hits        19174    19330     +156     
+ Misses       3340     3183     -157     
+ Partials      119      118       -1     

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

@eivindjahren
Copy link
Contributor Author

@frode-aarstad This cannot be merged currently though as it would break with current behavior that
summary keys from HISTORY_OBSERVATION are added to the summary config if missing.

if summary_key not in response_config.keys:
response_config.keys.append(summary_key)

@eivindjahren eivindjahren force-pushed the refactor_statefultest_to_allow_deletion branch from 728aea3 to 58cf54f Compare February 19, 2024 07:49
@eivindjahren eivindjahren force-pushed the refactor_statefultest_to_allow_deletion branch from 58cf54f to 0037825 Compare February 19, 2024 07:51
@eivindjahren eivindjahren marked this pull request as ready for review February 19, 2024 09:03
@eivindjahren eivindjahren added the release-notes:improvement Automatically categorise as improvement in release notes label Feb 19, 2024
@eivindjahren eivindjahren merged commit 9496957 into equinor:main Feb 19, 2024
53 of 54 checks passed
@eivindjahren eivindjahren deleted the refactor_statefultest_to_allow_deletion branch February 19, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:improvement Automatically categorise as improvement in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storage can create ensemble with state HAS_DATA when no data present
3 participants