Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12466 +/- ##
==========================================
- Coverage 90.69% 90.61% -0.09%
==========================================
Files 431 432 +1
Lines 29734 29738 +4
==========================================
- Hits 26968 26947 -21
- Misses 2766 2791 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
4a3d74d to
1480a97
Compare
62ca59e to
a7a3420
Compare
CodSpeed Performance ReportMerging #12466 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Pull request overview
This PR refactors parameter configuration handling by removing the ParameterMetadata class and making all parameter types use pydantic BaseModel directly with a dimensionality attribute. This simplifies the parameter configuration system while maintaining backward compatibility through storage migration.
Key Changes
- Removed
ParameterMetadataclass and replaced it with direct use of parameter configuration models - Added
dimensionalityfield directly to all parameter config classes (GenKwConfig, Field, SurfaceConfig) - Created storage migration
to19to add dimensionality to existing parameter configurations
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/ert/config/parameter_config.py |
Commented out ParameterMetadata class definition (should be removed entirely) |
src/ert/config/gen_kw_config.py |
Added dimensionality field (literal 1) and removed metadata property |
src/ert/config/field.py |
Added dimensionality field (literal 3) and removed metadata property |
src/ert/config/surface_config.py |
Added dimensionality field (literal 2) and removed metadata property |
src/ert/config/ext_param_config.py |
Removed metadata property |
src/ert/config/__init__.py |
Removed ParameterMetadata export |
src/ert/storage/migration/to19.py |
New migration to add dimensionality to existing parameters based on type |
src/ert/storage/local_storage.py |
Registered to19 migration |
src/ert/dark_storage/endpoints/experiments.py |
Changed from returning list of metadata to single config model_dump |
src/ert/dark_storage/endpoints/parameters.py |
Simplified parameter data fetching to use parameter name directly |
src/ert/gui/tools/plot/plot_api.py |
Updated to use parameter config instead of ParameterMetadata |
src/ert/gui/tools/plot/plot_window.py |
Updated to check dimensionality instead of data_origin metadata |
tests/ert/unit_tests/storage/test_storage_migration.py |
Added assertions to verify dimensionality after migration |
tests/ert/unit_tests/storage/snapshots/... |
Updated snapshots to include dimensionality field |
tests/ert/unit_tests/run_models/snapshots/... |
Updated experiment serialization snapshots with dimensionality |
tests/ert/unit_tests/gui/tools/plot/test_plot_api.py |
Updated tests to use GenKwConfig instead of ParameterMetadata |
tests/ert/unit_tests/gui/tools/plot/conftest.py |
Updated mock data to match new parameter structure |
tests/ert/unit_tests/dark_storage/test_parameters.py |
Removed test for group-based parameter access, simplified parameter lookup |
tests/ert/unit_tests/dark_storage/test_http_endpoints.py |
Updated tests to expect new parameter structure with full config |
Comments suppressed due to low confidence (1)
src/ert/config/parameter_config.py:46
- This comment appears to contain commented-out code.
# class ParameterMetadata(BaseModel):
# key: str
# transformation: str | None
# dimensionality: Literal[1, 2, 3] = 1
# userdata: dict[str, Any]
bf16d9d to
5bd1cee
Compare
|
Should there not be a |
Not sure. This is just adding a new attribute. Maybe I can make a simple one though |
frode-aarstad
left a comment
There was a problem hiding this comment.
Looks good. Squash and merge 🎅🏻
Since all parameters are pydantic now we don't need metadata class. This adds dimensionality attribute to parameters, which previously was part of the metadata. Additionally we remove group from the dark storage API. Add storage migration to account for dimensionality attribute
4871721 to
b3233c4
Compare
Issue
Resolves #12418
Make use that all parameters are pydantic basemodel now.
(Screenshot of new behavior in GUI if applicable)
It might require to change webviz-ert dark storage API
git rebase -i main --exec 'just rapid-tests')When applicable