From ea610376c5a7b4f9b06e585f9a75ddef135d5365 Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Sat, 8 Aug 2026 16:36:43 +0200 Subject: [PATCH 01/12] Remove unneeded quality control text fixtures --- .../data/valid/goodness_of_fit_extended.yaml | 6 ------ .../data/valid/missing_optional_sections.yaml | 3 --- 2 files changed, 9 deletions(-) delete mode 100644 src/wf_psf/tests/test_quality_control/data/valid/goodness_of_fit_extended.yaml delete mode 100644 src/wf_psf/tests/test_quality_control/data/valid/missing_optional_sections.yaml diff --git a/src/wf_psf/tests/test_quality_control/data/valid/goodness_of_fit_extended.yaml b/src/wf_psf/tests/test_quality_control/data/valid/goodness_of_fit_extended.yaml deleted file mode 100644 index b081e545..00000000 --- a/src/wf_psf/tests/test_quality_control/data/valid/goodness_of_fit_extended.yaml +++ /dev/null @@ -1,6 +0,0 @@ -metrics: - goodness_of_fit: - enabled: true - params: - inference_config: inference_config.yaml - model_cache: true \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/valid/missing_optional_sections.yaml b/src/wf_psf/tests/test_quality_control/data/valid/missing_optional_sections.yaml deleted file mode 100644 index 4a5fdba2..00000000 --- a/src/wf_psf/tests/test_quality_control/data/valid/missing_optional_sections.yaml +++ /dev/null @@ -1,3 +0,0 @@ -metrics: - mask_obscuration: - enabled: true \ No newline at end of file From bf56913bc28676db1b6de565d3aa2a95bc4f9c5e Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Sat, 8 Aug 2026 16:41:26 +0200 Subject: [PATCH 02/12] Update example quality control YAML file - Add resources section - Add required_resources option for metrics - Add params section with example parameters --- config/quality_control.yaml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/config/quality_control.yaml b/config/quality_control.yaml index 3f45b313..501543a4 100644 --- a/config/quality_control.yaml +++ b/config/quality_control.yaml @@ -1,12 +1,28 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + metrics: mask_obscuration: enabled: true + params: + aperture: gaussian + sigma: 2.5 goodness_of_fit: enabled: true + required_resources: + - psf_models.standard + params: - inference_config: inference_config.yaml + statistic: reduced_chi_square + normalize_residuals: true + rejection: @@ -21,4 +37,4 @@ rejection: reporting: save_metrics: true - log_statistics: true \ No newline at end of file + log_statistics: true \ No newline at end of file From efbfe16981ca27445999521760ebdca5f0e059c5 Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Sun, 9 Aug 2026 11:35:32 +0200 Subject: [PATCH 03/12] Update quality control configuration - Add dataclass ResourcesConfig to store resources available parameters - Add method parse_resources_config to parse resources config section - Add validators to check for internal consistency between resources, metrics and rejection params - Normalise doc string formats - Add missing doc strings for parser methods - Update changelog fragment re: quality control configuration framework --- ...quality_control_configuration_interface.md | 4 + src/wf_psf/quality_control/config.py | 272 ++++++++++++++++-- 2 files changed, 259 insertions(+), 17 deletions(-) diff --git a/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md b/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md index 8e1875ea..7d861dce 100644 --- a/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md +++ b/changelog.d/20260723_143802_jennifer.pollack_227_new_feature_add_quality_control_configuration_interface.md @@ -35,4 +35,8 @@ For top level release notes, leave all the headers commented out. configuration framework, including typed configuration objects, YAML parsing, and validation for quality metrics, rejection policies, and reporting. +- Extended the quality control configuration framework with configurable + resource declarations and metric resource requirements, including + validation of resource references and consistency between quality metrics + and rejection policies. diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index ce240563..702f463d 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -11,6 +11,7 @@ from __future__ import annotations from dataclasses import dataclass, field from pathlib import Path + from wf_psf.utils.read_config import read_yaml @@ -20,12 +21,19 @@ class QualityMetricConfig: Attributes ---------- - enabled: Whether the quality metric is enabled. - params: Arbitrary quality metric-specific parameters. + enabled : bool + Whether the quality metric is enabled. + + params : dict + Quality metric-specific parameters. + + required_resources : list[str] + Identifiers of resources required to compute the quality metric. """ enabled: bool = True params: dict = field(default_factory=dict) + required_resources: list[str] = field(default_factory=list) @dataclass @@ -34,8 +42,11 @@ class RejectionPolicyConfig: Attributes ---------- - enabled: Whether rejection policy is enabled. - threshold: Numeric threshold used to trigger rejection policy, or None. + enabled : bool + Whether rejection policy is enabled. + + threshold: float | None + Numeric threshold used by the rejection policy, or None if not applicable. """ enabled: bool = False @@ -48,23 +59,47 @@ class ReportingConfig: Attributes ---------- - save_metrics: Persist computed metrics to storage. - log_statistics: Emit statistics to the logging system. + save_metrics : bool + Persist computed metrics to storage. + + log_statistics : bool + Emit statistics to the logging system. """ save_metrics: bool = False log_statistics: bool = False +@dataclass +class ResourcesConfig: + """Configuration for resources available for quality control execution. + + Attributes + ---------- + available : dict + Mapping of resource types to configured resource identifiers and parameters. + """ + + available: dict = field(default_factory=dict) + + @dataclass class QualityControlConfig: """Top-level quality control configuration. Attributes ---------- - metrics: Mapping of metric name -> QualityMetricConfig. - rejection: Mapping of check name -> RejectionPolicyConfig. - reporting: ReportingConfig instance. + metrics : dict + Mapping of metric name to QualityMetricConfig instances. + + rejection : dict + Mapping of check name to RejectionPolicyConfig instances. + + reporting : ReportingConfig + ReportingConfig instance. + + resources : ResourcesConfig + Available resources used during quality control execution. """ metrics: dict[str, QualityMetricConfig] = field(default_factory=dict) @@ -73,17 +108,38 @@ class QualityControlConfig: reporting: ReportingConfig = field(default_factory=ReportingConfig) + resources: ResourcesConfig = field(default_factory=ResourcesConfig) + # config section parsers def parse_metrics_config( section: dict[str, dict] | None, ) -> dict[str, QualityMetricConfig]: - """Parse quality metric configuration section.""" + """Parse the quality metrics configuration section. + + Parameters + ---------- + section : dict[str, dict] or None + Raw quality metric configuration. If None, an empty configuration + is returned. + + Returns + ------- + dict[str, QualityMetricConfig] + Parsed quality metric configurations keyed by metric name. + + Raises + ------ + TypeError + If the configuration section or any metric configuration is not a + mapping, if ``enabled`` is not boolean, if ``params`` is not a + mapping, or if required resource identifiers are not strings. + """ if section is None: return {} if not isinstance(section, dict): - raise TypeError("Metrics configuration must be a mapping") + raise TypeError("Metrics configuration must be a mapping.") metrics = {} @@ -101,9 +157,15 @@ def parse_metrics_config( if not isinstance(params, dict): raise TypeError(f"Metric parameters for '{name}' must be a mapping.") + required_resources = cfg.get("required_resources", []) + + if not all(isinstance(resource, str) for resource in required_resources): + raise TypeError( + f"Required resources for metric '{name}' must contain only strings." + ) + metrics[name] = QualityMetricConfig( - enabled=enabled, - params=params, + enabled=enabled, params=params, required_resources=required_resources ) return metrics @@ -112,7 +174,25 @@ def parse_metrics_config( def parse_rejection_policy_config( config: dict[str, dict] | None, ) -> dict[str, RejectionPolicyConfig]: - """Parse rejection policy configuration section.""" + """Parse the rejection policy configuration section. + + Parameters + ---------- + config : dict[str, dict] or None + Raw rejection policy configuration. If None, an empty configuration + is returned. + + Returns + ------- + dict[str, RejectionPolicyConfig] + Parsed rejection policy configurations keyed by metric name. + + Raises + ------ + TypeError + If the configuration section or a policy configuration is not a + mapping. + """ if config is None: return {} @@ -133,7 +213,24 @@ def parse_rejection_policy_config( def parse_reporting_config(config: dict | None) -> ReportingConfig: - """Parse reporting configuration section.""" + """Parse the reporting configuration section. + + Parameters + ---------- + config : dict or None + Raw reporting configuration. If None, the default reporting + configuration is returned. + + Returns + ------- + ReportingConfig + Parsed reporting configuration. + + Raises + ------ + TypeError + If the configuration section is not a mapping. + """ if config is None: return ReportingConfig() @@ -143,10 +240,132 @@ def parse_reporting_config(config: dict | None) -> ReportingConfig: return ReportingConfig(**config) +def parse_resources_config( + config: dict | None, +) -> ResourcesConfig: + """Parse the resources configuration section. + + Parameters + ---------- + config : dict or None + Raw resource configuration. If None, an empty resource + configuration is returned. + + Returns + ------- + ResourcesConfig + Parsed resource configuration. + + Raises + ------ + TypeError + If the configuration section is not a mapping. + """ + if config is None: + return ResourcesConfig() + + if not isinstance(config, dict): + raise TypeError("Resources configuration must be a mapping.") + + return ResourcesConfig(available=config) + + +def validate_quality_control_config(config: QualityControlConfig) -> None: + """Validate internal consistency of a quality control configuration. + + Parameters + ---------- + config : QualityControlConfig + Parsed quality control configuration. + + Raises + ------ + ValueError + If any cross-section configuration dependency is invalid. + """ + validate_metric_resources(config) + validate_rejection_policy_metrics(config) + + +def validate_metric_resources(config: QualityControlConfig) -> None: + """Validate that all metric resource requirements can be resolved. + + Parameters + ---------- + config : QualityControlConfig + Parsed quality control configuration. + + Raises + ------ + ValueError + If a required resource identifier is malformed or references an + unavailable resource. + """ + for metric_name, metric in config.metrics.items(): + # loop over metric.required_resources + for req in metric.required_resources: + # Check if resource identifier has the correct form + if "." not in req: + raise ValueError( + f"Resource identifier '{req}' must have the form " + "'.'." + ) + # extract resource type (e.g. 'psf_models') and name (e.g. 'standard') + resource_type, resource_name = req.split(".", 1) + + # check compliance of resource identifier + if not resource_type or not resource_name: + raise ValueError( + f"Resource identifier '{req}' must have the form " + "'.'." + ) + + # check if resource_type is in config.resources.available + if resource_type not in config.resources.available: + raise ValueError( + f"Metric '{metric_name}' requires unknown resource '{req}'." + ) + + # check if resournce_name is in config.resource.available[resource_type] + if resource_name not in config.resources.available[resource_type]: + raise ValueError( + f"Metric '{metric_name}' requires unknown resource '{req}'." + ) + + +def validate_rejection_policy_metrics(config: QualityControlConfig) -> None: + """Validate rejection policies against configured quality metrics. + + Parameters + ---------- + config : QualityControlConfig + Parsed quality control configuration. + + Raises + ------ + ValueError + If an enabled rejection policy references an unknown or disabled + quality metric. + """ + for metric_name, metric_rejection_policy in config.rejection.items(): + if metric_rejection_policy.enabled: + # check that metric exists in config.metrics + if metric_name not in config.metrics: + raise ValueError( + f"Rejection policy configured for unknown metric '{metric_name}' " + ) + + if not config.metrics[metric_name].enabled: + raise ValueError( + f"Rejection policy cannot be enabled because metric '{metric_name}' is disabled." + ) + + SECTION_PARSERS = { "metrics": parse_metrics_config, "rejection": parse_rejection_policy_config, "reporting": parse_reporting_config, + "resources": parse_resources_config, } @@ -168,7 +387,22 @@ def __init__(self, qc_config_path: str | Path): self.qc_config_path = qc_config_path def load(self) -> QualityControlConfig: - """Load and parse configuration file.""" + """Load, parse, and validate the quality control configuration. + + Returns + ------- + QualityControlConfig + Parsed and validated quality control configuration. + + Raises + ------ + TypeError + If a configuration section has an invalid structure or type. + + ValueError + If the parsed configuration contains inconsistent references + between metrics, resources, or rejection policies. + """ qc_config = read_yaml(self.qc_config_path) config = {} @@ -176,4 +410,8 @@ def load(self) -> QualityControlConfig: values = qc_config.get(section, {}) config[section] = parser(values) - return QualityControlConfig(**config) + qc = QualityControlConfig(**config) + + validate_quality_control_config(qc) + + return qc From b0ab44f0f587556194cd6fb9adda16caf0c0e516 Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Sun, 9 Aug 2026 11:41:13 +0200 Subject: [PATCH 04/12] Update quality control config test module - Update/add config fixtures with resource sections - Update existing tests to evaluate resource sections - Add unit tests for parse resource section and validators - Add integration test for loader that runs validator --- .../tests/test_quality_control/config_test.py | 170 +++++++++++++++++- .../metric_resource_identifier_unknown.yaml | 15 ++ .../invalid/rejection_invalid_params.yaml | 5 +- .../invalid/rejection_metric_not_enabled.yaml | 22 +++ .../data/valid/quality_control.yaml | 23 ++- 5 files changed, 229 insertions(+), 6 deletions(-) create mode 100644 src/wf_psf/tests/test_quality_control/data/invalid/metric_resource_identifier_unknown.yaml create mode 100644 src/wf_psf/tests/test_quality_control/data/invalid/rejection_metric_not_enabled.yaml diff --git a/src/wf_psf/tests/test_quality_control/config_test.py b/src/wf_psf/tests/test_quality_control/config_test.py index 25b427eb..8b55ef23 100644 --- a/src/wf_psf/tests/test_quality_control/config_test.py +++ b/src/wf_psf/tests/test_quality_control/config_test.py @@ -6,6 +6,7 @@ """ +from contextlib import nullcontext as does_not_raise from pathlib import Path import pytest from wf_psf.quality_control.config import ( @@ -14,6 +15,12 @@ QualityMetricConfig, RejectionPolicyConfig, ReportingConfig, + ResourcesConfig, +) +from wf_psf.quality_control.config import ( + parse_resources_config, + validate_metric_resources, + validate_rejection_policy_metrics, ) @@ -22,16 +29,70 @@ def load_config(config_file: str) -> QualityControlConfig: return handler.load() +@pytest.fixture +def qc_config_factory(): + def factory( + *, + required_resources=None, + rejection_metric=None, + resources=None, + metrics=None, + rejection=None, + ): + return QualityControlConfig( + metrics=metrics + or { + "goodness_of_fit": QualityMetricConfig( + enabled=True, + required_resources=required_resources or [], + ) + }, + resources=resources + or ResourcesConfig( + available={ + "psf_models": { + "standard": { + "inference_config": "inference_standard.yaml", + } + } + } + ), + rejection=rejection + or { + rejection_metric or "goodness_of_fit": RejectionPolicyConfig( + enabled=True, + threshold=0.25, + ) + }, + ) + + return factory + + +# Test for config loading and parsers def test_quality_control_config_loading(): config = load_config("valid/quality_control.yaml") + assert isinstance(config.resources, ResourcesConfig) + assert "standard" in config.resources.available["psf_models"] + assert "oversampled" in config.resources.available["psf_models"] + assert ( + config.resources.available["psf_models"]["standard"]["inference_config"] + == "inference_standard.yaml" + ) + assert ( + config.resources.available["psf_models"]["oversampled"]["inference_config"] + == "inference_oversampled.yaml" + ) + assert "mask_obscuration" in config.metrics assert isinstance(config.metrics["mask_obscuration"], QualityMetricConfig) assert config.metrics["mask_obscuration"].enabled is True + assert config.metrics["mask_obscuration"].required_resources == [] assert isinstance(config.metrics["goodness_of_fit"], QualityMetricConfig) - assert config.metrics["goodness_of_fit"].params["inference_config"] == ( - "inference_config.yaml" + assert config.metrics["goodness_of_fit"].required_resources == ( + ["psf_models.standard"] ) assert isinstance(config.rejection["mask_obscuration"], RejectionPolicyConfig) @@ -41,6 +102,18 @@ def test_quality_control_config_loading(): assert config.reporting.save_metrics is True +def test_parse_resources_config(): + config = { + "psf_models": {"standard": {"inference_config": "inference_standard.yaml"}} + } + + resources = parse_resources_config(config) + + assert resources.available["psf_models"]["standard"]["inference_config"] == ( + "inference_standard.yaml" + ) + + def test_metrics_minimal(): config = load_config("valid/metric_minimal.yaml") @@ -77,3 +150,96 @@ def test_reporting_configuration_must_be_mapping(): match="Reporting configuration must be a mapping", ): load_config("invalid/reporting_invalid_type.yaml") + + +# Tests for validation methods +def test_validate_metric_resources_all_valid(qc_config_factory): + with does_not_raise(): + validate_metric_resources(qc_config_factory()) + + +def test_validate_metric_resources_invalid_identifier(qc_config_factory): + config = qc_config_factory(required_resources=["psf_model_standard"]) + with pytest.raises( + ValueError, + match="Resource identifier 'psf_model_standard' must have the form '.'.", + ): + validate_metric_resources(config) + + +def test_validate_metric_resources_unknown_resource_type(qc_config_factory): + config = qc_config_factory(required_resources=["images.segmentation_maps"]) + + with pytest.raises( + ValueError, + match="Metric 'goodness_of_fit' requires unknown resource 'images.segmentation_maps'.", + ): + validate_metric_resources(config) + + +def test_validate_metric_resources_unknown_resource_name(qc_config_factory): + config = qc_config_factory(required_resources=["psf_models.imaginary"]) + + with pytest.raises( + ValueError, + match="Metric 'goodness_of_fit' requires unknown resource 'psf_models.imaginary'.", + ): + validate_metric_resources(config) + + +def test_validate_metric_resources_empty_resource_name(qc_config_factory): + config = qc_config_factory(required_resources=["psf_models."]) + + with pytest.raises( + ValueError, + match="Resource identifier 'psf_models.' must have the form '.'.", + ): + validate_metric_resources(config) + + +def test_validate_rejection_policy_metrics_all_valid(qc_config_factory): + with does_not_raise(): + validate_rejection_policy_metrics(qc_config_factory()) + + +def test_validate_rejection_policy_metrics_metric_not_found(qc_config_factory): + config = qc_config_factory(rejection_metric="mask_obscuration") + + with pytest.raises( + ValueError, + match="Rejection policy configured for unknown metric 'mask_obscuration'.", + ): + validate_rejection_policy_metrics(config) + + +def test_validate_rejection_policy_metrics_metric_not_enabled(qc_config_factory): + metric = { + "goodness_of_fit": QualityMetricConfig( + enabled=False, + required_resources=[], + ) + } + + config = qc_config_factory(metrics=metric) + + with pytest.raises( + ValueError, + match="Rejection policy cannot be enabled because metric 'goodness_of_fit' is disabled.", + ): + validate_rejection_policy_metrics(config) + + +# Integration tests + + +def test_load_config_validates_configuration_pass(): + with does_not_raise(): + load_config("valid/quality_control.yaml") + + +def test_load_config_validates_configuration_raise_unknown_identifier(): + with pytest.raises( + ValueError, + match="Metric 'goodness_of_fit' requires unknown resource", + ): + load_config("invalid/metric_resource_identifier_unknown.yaml") diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/metric_resource_identifier_unknown.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/metric_resource_identifier_unknown.yaml new file mode 100644 index 00000000..50023177 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/metric_resource_identifier_unknown.yaml @@ -0,0 +1,15 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + +metrics: + + goodness_of_fit: + enabled: true + required_resources: + - images.segmentation_maps + diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml index e2165bf8..6d243baa 100644 --- a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml +++ b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml @@ -2,11 +2,10 @@ metrics: goodness_of_fit: enabled: true - inference_config: inference_config.yaml model_cache: true rejection: goodness_of_fit: enabled: true - inference_config: - threshold: -1 \ No newline at end of file + required_resources: + - images.segmentation_maps \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_metric_not_enabled.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_metric_not_enabled.yaml new file mode 100644 index 00000000..0dc25183 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_metric_not_enabled.yaml @@ -0,0 +1,22 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + +metrics: + + mask_obscuration: + enabled: false + params: + aperture: gaussian + sigma: 2.5 + + +rejection: + + mask_obscuration: + enabled: true + threshold: 0.25 diff --git a/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml b/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml index 4486774a..2cbb7a15 100644 --- a/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml +++ b/src/wf_psf/tests/test_quality_control/data/valid/quality_control.yaml @@ -1,12 +1,33 @@ +resources: + + psf_models: + standard: + inference_config: inference_standard.yaml + oversampled: + inference_config: inference_oversampled.yaml + metrics: mask_obscuration: enabled: true + params: + aperture: gaussian + sigma: 2.5 goodness_of_fit: enabled: true + required_resources: + - psf_models.standard + params: - inference_config: inference_config.yaml + statistic: reduced_chi_square + normalize_residuals: true + + shapes: + enabled: false + required_resources: + - psf_models.oversampled + rejection: From e1e92bafa1f9c759bd23f3a96431dee41f9896cb Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Wed, 12 Aug 2026 12:40:24 +0200 Subject: [PATCH 05/12] Fix doc string format --- src/wf_psf/quality_control/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index 702f463d..62b96efa 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -45,7 +45,7 @@ class RejectionPolicyConfig: enabled : bool Whether rejection policy is enabled. - threshold: float | None + threshold : float | None Numeric threshold used by the rejection policy, or None if not applicable. """ From a765660a8204522604292607406f4d8952fa89f5 Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Wed, 12 Aug 2026 12:49:03 +0200 Subject: [PATCH 06/12] Fix typo in the doc string --- src/wf_psf/quality_control/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index 62b96efa..c70f1aaf 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -93,7 +93,7 @@ class QualityControlConfig: Mapping of metric name to QualityMetricConfig instances. rejection : dict - Mapping of check name to RejectionPolicyConfig instances. + Mapping of rejection policy name to RejectionPolicyConfig instances. reporting : ReportingConfig ReportingConfig instance. From 2af66792ac18893c4ba20a8be99f145c4b59efa7 Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Wed, 12 Aug 2026 13:00:44 +0200 Subject: [PATCH 07/12] Consolidate conditional checks on existence of requir ed resources --- src/wf_psf/quality_control/config.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index c70f1aaf..e077bc9b 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -302,32 +302,27 @@ def validate_metric_resources(config: QualityControlConfig) -> None: unavailable resource. """ for metric_name, metric in config.metrics.items(): - # loop over metric.required_resources for req in metric.required_resources: - # Check if resource identifier has the correct form if "." not in req: raise ValueError( f"Resource identifier '{req}' must have the form " "'.'." ) - # extract resource type (e.g. 'psf_models') and name (e.g. 'standard') + resource_type, resource_name = req.split(".", 1) - # check compliance of resource identifier if not resource_type or not resource_name: raise ValueError( f"Resource identifier '{req}' must have the form " "'.'." ) - # check if resource_type is in config.resources.available - if resource_type not in config.resources.available: - raise ValueError( - f"Metric '{metric_name}' requires unknown resource '{req}'." - ) + resources = config.resources.available - # check if resournce_name is in config.resource.available[resource_type] - if resource_name not in config.resources.available[resource_type]: + if ( + resource_type not in resources + or resource_name not in resources[resource_type] + ): raise ValueError( f"Metric '{metric_name}' requires unknown resource '{req}'." ) From db29127c60261c550348d4378b2bdb1977de197f Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Mon, 17 Aug 2026 14:45:49 +0200 Subject: [PATCH 08/12] Generalize configuration section parsers to accept Mapping - Accept Mapping inputs instead of concrete dictionaries. - Normalize mapped configuration data to dictionaries in the parsed configuration. - Update parser terminology and docstrings accordingly. --- src/wf_psf/quality_control/config.py | 49 +++++++++++++++------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index e077bc9b..2cc6fb83 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -9,8 +9,10 @@ """ from __future__ import annotations +from collections.abc import Mapping from dataclasses import dataclass, field from pathlib import Path +from typing import Any from wf_psf.utils.read_config import read_yaml @@ -80,7 +82,7 @@ class ResourcesConfig: Mapping of resource types to configured resource identifiers and parameters. """ - available: dict = field(default_factory=dict) + available: dict[str, Any] = field(default_factory=dict) @dataclass @@ -113,13 +115,13 @@ class QualityControlConfig: # config section parsers def parse_metrics_config( - section: dict[str, dict] | None, + section: Mapping[str, Any] | None, ) -> dict[str, QualityMetricConfig]: """Parse the quality metrics configuration section. Parameters ---------- - section : dict[str, dict] or None + section : Mapping[str, Any] or None Raw quality metric configuration. If None, an empty configuration is returned. @@ -133,18 +135,19 @@ def parse_metrics_config( TypeError If the configuration section or any metric configuration is not a mapping, if ``enabled`` is not boolean, if ``params`` is not a - mapping, or if required resource identifiers are not strings. + mapping, or if required resource identifiers is not a list, or if + its entries are not strings. """ if section is None: return {} - if not isinstance(section, dict): + if not isinstance(section, Mapping): raise TypeError("Metrics configuration must be a mapping.") metrics = {} for name, cfg in section.items(): - if not isinstance(cfg, dict): + if not isinstance(cfg, Mapping): raise TypeError(f"Metric configuration '{name}' must be a mapping.") enabled = cfg.get("enabled", True) @@ -154,7 +157,7 @@ def parse_metrics_config( params = cfg.get("params", {}) - if not isinstance(params, dict): + if not isinstance(params, Mapping): raise TypeError(f"Metric parameters for '{name}' must be a mapping.") required_resources = cfg.get("required_resources", []) @@ -165,20 +168,20 @@ def parse_metrics_config( ) metrics[name] = QualityMetricConfig( - enabled=enabled, params=params, required_resources=required_resources + enabled=enabled, params=dict(params), required_resources=required_resources ) return metrics def parse_rejection_policy_config( - config: dict[str, dict] | None, + section: Mapping[str, Any] | None, ) -> dict[str, RejectionPolicyConfig]: """Parse the rejection policy configuration section. Parameters ---------- - config : dict[str, dict] or None + section : Mapping[str, Any] or None Raw rejection policy configuration. If None, an empty configuration is returned. @@ -193,16 +196,16 @@ def parse_rejection_policy_config( If the configuration section or a policy configuration is not a mapping. """ - if config is None: + if section is None: return {} - if not isinstance(config, dict): + if not isinstance(section, Mapping): raise TypeError("Rejection policy configuration must be a mapping.") policies = {} - for name, cfg in config.items(): - if not isinstance(cfg, dict): + for name, cfg in section.items(): + if not isinstance(cfg, Mapping): raise TypeError( f"Rejection policy configuration '{name}' must be a mapping." ) @@ -212,12 +215,12 @@ def parse_rejection_policy_config( return policies -def parse_reporting_config(config: dict | None) -> ReportingConfig: +def parse_reporting_config(section: Mapping[str, Any] | None) -> ReportingConfig: """Parse the reporting configuration section. Parameters ---------- - config : dict or None + section : dict or None Raw reporting configuration. If None, the default reporting configuration is returned. @@ -231,23 +234,23 @@ def parse_reporting_config(config: dict | None) -> ReportingConfig: TypeError If the configuration section is not a mapping. """ - if config is None: + if section is None: return ReportingConfig() - if not isinstance(config, dict): + if not isinstance(section, Mapping): raise TypeError("Reporting configuration must be a mapping.") - return ReportingConfig(**config) + return ReportingConfig(**section) def parse_resources_config( - config: dict | None, + config: Mapping[str, Any] | None, ) -> ResourcesConfig: """Parse the resources configuration section. Parameters ---------- - config : dict or None + config : Mapping or None Raw resource configuration. If None, an empty resource configuration is returned. @@ -264,10 +267,10 @@ def parse_resources_config( if config is None: return ResourcesConfig() - if not isinstance(config, dict): + if not isinstance(config, Mapping): raise TypeError("Resources configuration must be a mapping.") - return ResourcesConfig(available=config) + return ResourcesConfig(available=dict(config)) def validate_quality_control_config(config: QualityControlConfig) -> None: From 5ca828cc7aef8455150d5a5e13cf3ab2e7f01c25 Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Mon, 17 Aug 2026 15:38:25 +0200 Subject: [PATCH 09/12] Consolidate and extend validation checks on metric resources - Simplify resource identifier validation into a single conditional. - Reject identifiers containing more than one "." separator. - Consolidate identifier validation tests using pytest.mark.parametrize. - Consolidate unknown resource validation tests using pytest.mark.parametrize. --- src/wf_psf/quality_control/config.py | 11 +--- .../tests/test_quality_control/config_test.py | 58 +++++++++++-------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index 2cc6fb83..d75b98fc 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -306,20 +306,15 @@ def validate_metric_resources(config: QualityControlConfig) -> None: """ for metric_name, metric in config.metrics.items(): for req in metric.required_resources: - if "." not in req: - raise ValueError( - f"Resource identifier '{req}' must have the form " - "'.'." - ) - - resource_type, resource_name = req.split(".", 1) + parts = req.split(".") - if not resource_type or not resource_name: + if len(parts) != 2 or not all(parts): raise ValueError( f"Resource identifier '{req}' must have the form " "'.'." ) + resource_type, resource_name = parts resources = config.resources.available if ( diff --git a/src/wf_psf/tests/test_quality_control/config_test.py b/src/wf_psf/tests/test_quality_control/config_test.py index 8b55ef23..85d8a3d2 100644 --- a/src/wf_psf/tests/test_quality_control/config_test.py +++ b/src/wf_psf/tests/test_quality_control/config_test.py @@ -158,41 +158,49 @@ def test_validate_metric_resources_all_valid(qc_config_factory): validate_metric_resources(qc_config_factory()) -def test_validate_metric_resources_invalid_identifier(qc_config_factory): - config = qc_config_factory(required_resources=["psf_model_standard"]) - with pytest.raises( - ValueError, - match="Resource identifier 'psf_model_standard' must have the form '.'.", - ): - validate_metric_resources(config) - - -def test_validate_metric_resources_unknown_resource_type(qc_config_factory): - config = qc_config_factory(required_resources=["images.segmentation_maps"]) - - with pytest.raises( - ValueError, - match="Metric 'goodness_of_fit' requires unknown resource 'images.segmentation_maps'.", - ): - validate_metric_resources(config) - - -def test_validate_metric_resources_unknown_resource_name(qc_config_factory): - config = qc_config_factory(required_resources=["psf_models.imaginary"]) +@pytest.mark.parametrize( + "required_resource", + [ + "psf_model_standard", + "psf_models.standard.foo", + "psf_models.", + ".standard", + ], +) +def test_validate_metric_resources_invalid_identifier( + qc_config_factory, + required_resource, +): + config = qc_config_factory(required_resources=[required_resource]) with pytest.raises( ValueError, - match="Metric 'goodness_of_fit' requires unknown resource 'psf_models.imaginary'.", + match=( + f"Resource identifier '{required_resource}' must have the form " + "'.'." + ), ): validate_metric_resources(config) -def test_validate_metric_resources_empty_resource_name(qc_config_factory): - config = qc_config_factory(required_resources=["psf_models."]) +@pytest.mark.parametrize( + "required_resource", + [ + "images.segmentation_maps", + "psf_models.imaginary", + ], +) +def test_validate_metric_resources_unknown_resource( + qc_config_factory, + required_resource, +): + config = qc_config_factory(required_resources=[required_resource]) with pytest.raises( ValueError, - match="Resource identifier 'psf_models.' must have the form '.'.", + match=( + f"Metric 'goodness_of_fit' requires unknown resource '{required_resource}'." + ), ): validate_metric_resources(config) From 5b2654a646cafe8ac26193cf60db758f8c62dbb8 Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Mon, 17 Aug 2026 15:51:06 +0200 Subject: [PATCH 10/12] Use early continue for disabled rejection policies --- src/wf_psf/quality_control/config.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index d75b98fc..98576ec6 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -341,17 +341,18 @@ def validate_rejection_policy_metrics(config: QualityControlConfig) -> None: quality metric. """ for metric_name, metric_rejection_policy in config.rejection.items(): - if metric_rejection_policy.enabled: - # check that metric exists in config.metrics - if metric_name not in config.metrics: - raise ValueError( - f"Rejection policy configured for unknown metric '{metric_name}' " - ) + if not metric_rejection_policy.enabled: + continue - if not config.metrics[metric_name].enabled: - raise ValueError( - f"Rejection policy cannot be enabled because metric '{metric_name}' is disabled." - ) + if metric_name not in config.metrics: + raise ValueError( + f"Rejection policy configured for unknown metric '{metric_name}' " + ) + + if not config.metrics[metric_name].enabled: + raise ValueError( + f"Rejection policy cannot be enabled because metric '{metric_name}' is disabled." + ) SECTION_PARSERS = { From 1359016e5a92da3278c3ffb6fa23471137f31cda Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Mon, 17 Aug 2026 16:18:06 +0200 Subject: [PATCH 11/12] Define defaults in qc_config_factory for readability --- .../tests/test_quality_control/config_test.py | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/wf_psf/tests/test_quality_control/config_test.py b/src/wf_psf/tests/test_quality_control/config_test.py index 85d8a3d2..b00f362d 100644 --- a/src/wf_psf/tests/test_quality_control/config_test.py +++ b/src/wf_psf/tests/test_quality_control/config_test.py @@ -39,31 +39,34 @@ def factory( metrics=None, rejection=None, ): - return QualityControlConfig( - metrics=metrics - or { - "goodness_of_fit": QualityMetricConfig( - enabled=True, - required_resources=required_resources or [], - ) - }, - resources=resources - or ResourcesConfig( - available={ - "psf_models": { - "standard": { - "inference_config": "inference_standard.yaml", - } + metric_default = { + "goodness_of_fit": QualityMetricConfig( + enabled=True, + required_resources=required_resources or [], + ) + } + + resources_default = ResourcesConfig( + available={ + "psf_models": { + "standard": { + "inference_config": "inference_standard.yaml", } } - ), - rejection=rejection - or { - rejection_metric or "goodness_of_fit": RejectionPolicyConfig( - enabled=True, - threshold=0.25, - ) - }, + } + ) + + rejection_default = { + rejection_metric or "goodness_of_fit": RejectionPolicyConfig( + enabled=True, + threshold=0.25, + ) + } + + return QualityControlConfig( + metrics=metric_default if metrics is None else metrics, + resources=resources_default if resources is None else resources, + rejection=rejection_default if rejection is None else rejection, ) return factory From 17a099a39e5fada58b94600094b80f505895f2fc Mon Sep 17 00:00:00 2001 From: Jennifer Pollack Date: Mon, 17 Aug 2026 16:49:23 +0200 Subject: [PATCH 12/12] Add type check that required_resources is a list - Update parse_metrics_config to require required_resources to be a list. - Add a TypeError test and fixture for a non-list required_resources value. - Add a TypeError test and fixture for a non-string required_resources element. - Remove the deprecated fixture YAML file. --- src/wf_psf/quality_control/config.py | 3 +++ .../tests/test_quality_control/config_test.py | 16 ++++++++++++++++ ...etric_required_resources_invalid_element.yaml | 6 ++++++ .../metric_required_resources_invalid_type.yaml | 4 ++++ .../data/invalid/rejection_invalid_params.yaml | 11 ----------- 5 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_element.yaml create mode 100644 src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_type.yaml delete mode 100644 src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml diff --git a/src/wf_psf/quality_control/config.py b/src/wf_psf/quality_control/config.py index 98576ec6..b9e9e074 100644 --- a/src/wf_psf/quality_control/config.py +++ b/src/wf_psf/quality_control/config.py @@ -162,6 +162,9 @@ def parse_metrics_config( required_resources = cfg.get("required_resources", []) + if not isinstance(required_resources, list): + raise TypeError(f"Required resources for metric '{name}' must be a list.") + if not all(isinstance(resource, str) for resource in required_resources): raise TypeError( f"Required resources for metric '{name}' must contain only strings." diff --git a/src/wf_psf/tests/test_quality_control/config_test.py b/src/wf_psf/tests/test_quality_control/config_test.py index b00f362d..e06668c8 100644 --- a/src/wf_psf/tests/test_quality_control/config_test.py +++ b/src/wf_psf/tests/test_quality_control/config_test.py @@ -117,6 +117,22 @@ def test_parse_resources_config(): ) +def test_required_resources_must_be_a_list(): + with pytest.raises( + TypeError, + match="Required resources for metric 'goodness_of_fit' must be a list.", + ): + load_config("invalid/metric_required_resources_invalid_type.yaml") + + +def test_required_resources_element_must_be_a_str(): + with pytest.raises( + TypeError, + match="Required resources for metric 'goodness_of_fit' must contain only strings.", + ): + load_config("invalid/metric_required_resources_invalid_element.yaml") + + def test_metrics_minimal(): config = load_config("valid/metric_minimal.yaml") diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_element.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_element.yaml new file mode 100644 index 00000000..6e91f023 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_element.yaml @@ -0,0 +1,6 @@ +metrics: + goodness_of_fit: + enabled: true + required_resources: + - psf_models.standard + - 123 \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_type.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_type.yaml new file mode 100644 index 00000000..053a38d1 --- /dev/null +++ b/src/wf_psf/tests/test_quality_control/data/invalid/metric_required_resources_invalid_type.yaml @@ -0,0 +1,4 @@ +metrics: + goodness_of_fit: + enabled: true + required_resources: psf_models.standard \ No newline at end of file diff --git a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml b/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml deleted file mode 100644 index 6d243baa..00000000 --- a/src/wf_psf/tests/test_quality_control/data/invalid/rejection_invalid_params.yaml +++ /dev/null @@ -1,11 +0,0 @@ -# Future test data for validation of configuration settings -metrics: - goodness_of_fit: - enabled: true - model_cache: true - -rejection: - goodness_of_fit: - enabled: true - required_resources: - - images.segmentation_maps \ No newline at end of file