Remove local dataset notion #2645
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BjarneHerland
force-pushed
the
remove_local_dataset_notion
branch
8 times, most recently
from
51b8bac
to
002f5b2
Compare
Codecov Report
@@ Coverage Diff @@
## main #2645 +/- ##
==========================================
+ Coverage 64.77% 64.82% +0.04%
==========================================
Files 650 649 -1
Lines 54063 53876 -187
Branches 4591 4547 -44
==========================================
- Hits 35021 34924 -97
+ Misses 17562 17493 -69
+ Partials 1480 1459 -21
Continue to review full report at Codecov.
|
BjarneHerland
force-pushed
the
remove_local_dataset_notion
branch
from
002f5b2
to
3ac6128
Compare
ManInFez
reviewed
Jan 5, 2022
BjarneHerland
force-pushed
the
remove_local_dataset_notion
branch
from
3ac6128
to
549453e
Compare
|
Remember to rebase and clean up the commit messages |
ManInFez
reviewed
Jan 5, 2022
res/enkf/local_ministep.py
Outdated
| raise TypeError("Keys must be strings, not int!") | ||
| if data_key in self: | ||
| return self._get_local_data(data_key) | ||
| def setEnsembleConfig(self, config): |
There was a problem hiding this comment.
perhaps we should avoid camel-casing?
This type was relevant when using Kalman-filters which is not the case anymore. See equinor#2539 for details. Some parts of the functionality was needed, however, so the class can not just be removed. Move necessary functionality from local_dataset_type to local_ministep_type, then update and simplify usage. Separate unit-test for local_dataset_type removed.
BjarneHerland
force-pushed
the
remove_local_dataset_notion
branch
from
b92282b
to
c0a4b76
Compare
|
test ert please |
1 similar comment
|
test ert please |
ManInFez
approved these changes
Jan 6, 2022
oyvindeide
added
the
release-notes:breaking-change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Resolves #2539
Approach
Moving functionality from
local_dataset_typetolocal_ministep_type, the remove the former.PR currently has two commits: first to make
librescompile and run ctests, second to fix some issues and make the Python-based tests work. There will be at third commit to remove all traces oflocal_dataset_typebut I expect a number of comments and suggestions on the first two to handle first.