Remove DESIGN_MATRIX_GROUP for parameter_configuration in design matrix #9840
Conversation
xjules
added
the
release-notes:skip
CodSpeed Performance ReportMerging #9840 will not alter performanceComparing Summary
|
|
Will we have problems due to duplicate keys if we flatten the dict, before under DESIGN_MATRIX_GROUP? |
Not sure what you mean by this? This only removes the parameter_configuration as dict or you meant something else? |
I misunderstood what was meant by DESIGN_MATRIX_GROUP in this context. I will look over it again 👍 |
| @@ -77,7 +77,7 @@ def test_merge_multiple_occurrences( | |||
| design_matrix_1.merge_with_other(design_matrix_2) | |||
| else: | |||
| design_matrix_1.merge_with_other(design_matrix_2) | |||
| design_params = design_matrix_1.parameter_configuration.get("DESIGN_MATRIX", []) | |||
| design_params = design_matrix_1.parameter_configuration | |||
There was a problem hiding this comment.
Can parameter_configuration be None? Maybe we should add or [] at the end?
There was a problem hiding this comment.
then the test will fail. I can do assert though....
src/ert/config/design_matrix.py
Outdated
| pc_other = dm_other.parameter_configuration[DESIGN_MATRIX_GROUP] | ||
| pc_self = self.parameter_configuration[DESIGN_MATRIX_GROUP] | ||
| pc_other = dm_other.parameter_configuration | ||
| pc_self = self.parameter_configuration | ||
| assert isinstance(pc_other, GenKwConfig) |
There was a problem hiding this comment.
Are you able to remove these asserts now? parameter_configuration is now simply a GenKwConfig, and so the type hints should be changed if possible in a couple of places? Maybe the name should be changed as well.
The dictionary introduced in parameter configuration in design matrix config seems to be not necessary and thus removing it. The type will those become GenKwConfig directly.
Issue
Resolves #9718
Approach
The dictionary introduced in parameter configuration in design matrix config seems not to be necessary. ✂️
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'pytest tests/ert/unit_tests tests/everest -n auto --hypothesis-profile=fast -m "not integration_test"')When applicable