Repository navigation
Refactor channels config - #451
Conversation
| return self | ||
|
|
||
| @model_validator(mode="after") | ||
| def check_all_channels_have_normalisation_constants(self) -> "Satellite": |
There was a problem hiding this comment.
We get this for free now with the new config format
| return _validate_grid_resolution_minutes(resolution) | ||
|
|
||
| @model_validator(mode="after") | ||
| def validate_accum_channels_subset(self) -> "NWP": |
There was a problem hiding this comment.
We get this for free now with the new config format
| return provider | ||
|
|
||
| @model_validator(mode="after") | ||
| def check_all_channels_have_normalisation_constants(self) -> "NWP": |
There was a problem hiding this comment.
We get this for free now with the new config format
| @field_validator("channels") | ||
| def validate_channel_names(cls, channels: list[Channel]) -> list[Channel]: | ||
| """Reject duplicate source channel names.""" | ||
| _validate_unique_channel_names(channels) |
There was a problem hiding this comment.
We add extra validation that all the channel names are unique
| if config.satellite is not None: | ||
| sat_config = config.satellite | ||
| normalisation_arrays["sat"] = _build_source_normalisation_arrays( | ||
| [sat_config.normalisation_constants[channel] for channel in sat_config.channels] |
There was a problem hiding this comment.
This is cleaner now since the normalisation constants are attached to the channels themselves
|
|
||
| class NormalisationConstantsMixin(Base): | ||
| """Normalisation constants for multiple channels.""" | ||
| class NWPChannel(Channel): |
There was a problem hiding this comment.
These get their own class since we support optionally differencing NWP channels but not satellite channels
| _validate_configuration(config) | ||
|
|
||
|
|
||
| def test_accum_channels_validation(config_filename): |
There was a problem hiding this comment.
Its now impossible to misconfigure the accumulated channels in the way this test covered
| def test_save_load_yaml_configuration(tmp_path, config_filename): | ||
| """Make sure a saved configuration is the same after loading""" | ||
|
|
||
| # Start with this config |
There was a problem hiding this comment.
I stripped out these unnecessary seeming comments
|
|
||
| def test_normalise_channels(config_filename): | ||
| config = load_yaml_configuration(config_filename) | ||
| config.satellite.channels = ["b", "a"] |
There was a problem hiding this comment.
These changes just directly maps these settings to the new config schema
felix-e-h-p
left a comment
felix-e-h-p
left a comment
There was a problem hiding this comment.
Small one. Could we mention in the mean/std field descriptions (not just the NWPChannel docstring) that for accumulated: true these apply to the differenced values? Otherwise looks great, nice one!
I don't think we should do it on EDIT: I've added to the description of |
|
Good to go, just the conflicts - thanks! |
I'm waiting on part_22 to be approved and merged, then I'll squash it onto dev and I'll rebase and squash this and all the others onto dev |
Pull Request
Description
This PR refactors the channels part of the config. Previously we had multiple entries in the config to specify the selection and processing of the channels. We had separate entries to define the list of channels used, their normalisation constants, and which of them should be diffed.
This PR wraps those together so the channels are configured in one entry. This removes some of the validation we need, and removes the users requirement to duplicate the list of channels between channels selected, channels diffed, and the channel normalisation stats.
Checklist: