diff --git a/pyaml/configuration/factory.py b/pyaml/configuration/factory.py index 0035cb12..ecaaba4a 100644 --- a/pyaml/configuration/factory.py +++ b/pyaml/configuration/factory.py @@ -8,6 +8,8 @@ from ..common.element import Element from ..common.exception import PyAMLConfigException +from ..validation.errors import raise_validation_error +from ..validation.schema_builder import generate_class_path from .unbound_element import UnboundElement # --------------------------------------------------------------------- @@ -284,9 +286,10 @@ def _build_object(self, data: dict, ignore_external: bool = False): try: cfg = build_info.config_cls.model_validate(config) except ValidationError as exc: - raise PyAMLConfigException( - f"Validation failed for {build_info.config_cls.__module__}.{build_info.config_cls.__name__}:\n{exc}" - ) from exc + raise_validation_error( + exc, + class_path=generate_class_path(build_info.config_cls), + ) else: cfg = config diff --git a/pyaml/validation/errors.py b/pyaml/validation/errors.py index 8386d072..fb765c8b 100644 --- a/pyaml/validation/errors.py +++ b/pyaml/validation/errors.py @@ -1,7 +1,7 @@ """Functionality for attaching location information to validation errors.""" from dataclasses import dataclass -from typing import Any +from typing import Any, NoReturn from pydantic import ValidationError @@ -13,8 +13,14 @@ class Location: """ Source location within a configuration file. - Stores the file name together with the line and column at which a - configuration object or field was defined. + Parameters + ---------- + file : str + Name of the configuration file. + line : int + Line number where the object or field was defined. + column : int + Column number where the object or field was defined. """ file: str @@ -22,7 +28,7 @@ class Location: column: int def __str__(self) -> str: - return f"{self.file} at line {self.line}, column {self.column}." + return f"{self.file}: line {self.line}, column {self.column}" @dataclass(frozen=True) @@ -30,8 +36,12 @@ class LocationMetadata: """ Location metadata extracted from configuration data. - Stores the source location of a configuration object together with - optional locations for individual configuration fields. + Parameters + ---------- + location : Location | None + Source location of the configuration object itself. + field_locations : dict[str, Location] | None, optional + Source locations for individual configuration fields. """ location: Location | None @@ -42,8 +52,16 @@ def extract_location_metadata(data: dict[str, Any]) -> tuple[dict[str, Any], Loc """ Extract loader-added location metadata from configuration data. - Returns a copy of the configuration dictionary with the metadata - removed together with the extracted location information. + Parameters + ---------- + data : dict[str, Any] + Configuration data potentially containing loader-added metadata. + + Returns + ------- + tuple[dict[str, Any], LocationMetadata] + A copy of the configuration dictionary with the metadata removed, + together with the extracted location information. """ cleaned = dict(data) @@ -66,48 +84,114 @@ def extract_location_metadata(data: dict[str, Any]) -> tuple[dict[str, Any], Loc ) +def _format_value(value: Any, max_len: int = 120) -> str: + """ + Format a value for inclusion in an error message. + + Parameters + ---------- + value : Any + Value to format. + max_len : int, optional + Maximum length of the formatted representation, by default 120. + + Returns + ------- + str + Formatted value string. + """ + + text = repr(value) + return text if len(text) <= max_len else text[: max_len - 3] + "..." + + +def _format_location_path(loc: tuple[Any, ...]) -> str: + """ + Format a Pydantic error location as a human-readable path. + + Parameters + ---------- + loc : tuple[Any, ...] + Location tuple from a Pydantic validation error. + + Returns + ------- + str + Human-readable location path such as ``items[0].name``. + Returns ```` for an empty location. + """ + + parts: list[str] = [] + + for item in loc: + if isinstance(item, int): + if parts: + parts[-1] = f"{parts[-1]}[{item}]" + else: + parts.append(f"[{item}]") + else: + parts.append(str(item)) + + return ".".join(parts) if parts else "" + + def raise_validation_error( exc: ValidationError, class_path: str, location_metadata: LocationMetadata | None = None, -) -> None: +) -> NoReturn: """ Raise a configuration exception from a Pydantic validation error. - Validation messages are formatted into a human-readable error message. - If location metadata is available, source locations for the - configuration object and its fields are included in the reported - error. + Parameters + ---------- + exc : ValidationError + Validation error raised by Pydantic. + class_path : str + Fully qualified class path of the configuration object being validated. + location_metadata : LocationMetadata | None, optional + Source location metadata extracted from the configuration data, by + default None. + + Raises + ------ + PyAMLConfigException + Always raised with a formatted human-readable error message. """ - messages: list[str] = [] + header = [f"Validation failed for class: '{class_path}'"] + + if location_metadata is not None and location_metadata.location is not None: + header.append(f"at {location_metadata.location}.") + + else: + header[-1] += "." + + error_lines: list[str] = [] for err in exc.errors(): - loc = err.get("loc", ()) + loc = tuple(err.get("loc", ())) msg = err["msg"] + bad_value = err.get("input", None) - if len(loc) == 2: - field, field_idx = loc - message = f"'{field}.{field_idx}': {msg}" - field_name = field - elif len(loc) == 1: - field_name = loc[0] - message = f"'{field_name}': {msg}" - else: - field_name = None - message = f"{loc}: {msg}" + path = _format_location_path(loc) + error_lines.append(f"Field '{path}' is invalid:") + error_lines.append(f" error: {msg}") + + if bad_value is not None: + error_lines.append(f" got: {_format_value(bad_value)}") + field_name = loc[0] if loc else None if ( location_metadata is not None and location_metadata.field_locations is not None and field_name in location_metadata.field_locations ): - message += f" ({location_metadata.field_locations[field_name]})" + error_lines.append(f" location: {location_metadata.field_locations[field_name]}") - messages.append(message) - - location_str = "" - if location_metadata is not None and location_metadata.location is not None: - location_str = f" ({location_metadata.location})" + if header[-1].endswith("."): + message = "\n".join(header + error_lines) + else: + message = f"{header[0]} {' '.join(header[1:])} {error_lines[0]}\n" + "\n".join(error_lines[1:]) - raise PyAMLConfigException(f"{'; '.join(messages)} for class: '{class_path}'{location_str}") from None + raise PyAMLConfigException(message) from None diff --git a/pyaml/validation/validation_models.py b/pyaml/validation/validation_models.py index 1cd9e8bc..ccde0904 100644 --- a/pyaml/validation/validation_models.py +++ b/pyaml/validation/validation_models.py @@ -3,12 +3,13 @@ import inspect import logging from abc import ABCMeta -from typing import Any +from typing import Any, cast -from pydantic import BaseModel, ConfigDict, create_model +from pydantic import BaseModel, ConfigDict, ValidationError, create_model from .configuration_models import PyAMLBaseModel -from .schema_builder import _fields_from_constructor_signature +from .errors import raise_validation_error +from .schema_builder import _fields_from_constructor_signature, generate_class_path logger = logging.getLogger(__name__) @@ -41,25 +42,27 @@ def __call__(cls, *args: Any, **kwargs: Any): """ Create an instance after optionally validating constructor arguments. - The supplied arguments are bound to the class ``__init__`` signature, - default values are applied, and the resulting argument mapping is - validated using ``validation_model`` unless ``validate=False`` is - passed to the constructor. The validated values are then passed to the - constructor. - Parameters ---------- - validate + *args : Any + Positional constructor arguments. + **kwargs : Any + Keyword constructor arguments. + validate : bool, optional If ``True`` (default), validate constructor arguments before - instantiation. If ``False``, skip validation and pass the supplied - arguments directly to the constructor. + instantiation. If ``False``, skip validation and pass the + supplied arguments directly to the constructor. + + Returns + ------- + object + Instance of the class after validation and construction. Raises ------ TypeError If the class does not define ``validation_model``. - - ValidationError + PyAMLConfigException If the supplied arguments do not conform to the validation model. """ @@ -88,7 +91,14 @@ def __call__(cls, *args: Any, **kwargs: Any): # Validate the model logger.debug("Validating input against schema: %s", validation_model.model_fields) - validated = validation_model.model_validate(arguments) + + try: + validated = validation_model.model_validate(arguments) + except ValidationError as exc: + raise_validation_error( + exc, + class_path=generate_class_path(cls), + ) # Return the object return super().__call__(**validated.model_dump()) @@ -112,10 +122,15 @@ def __init_subclass__(cls, **kwargs): """ Generate and attach a validation model for the subclass. - A validation model is generated from the subclass's constructor - signature and assigned to ``validation_model``. Defining - ``validation_model`` explicitly is not permitted and results in a - :class:`TypeError`. + Parameters + ---------- + **kwargs : Any + Additional keyword arguments passed to ``super().__init_subclass__``. + + Raises + ------ + TypeError + If ``validation_model`` is defined manually on the subclass. """ super().__init_subclass__(**kwargs) @@ -144,9 +159,9 @@ def _build_validation_model(cls) -> type[ValidationModel]: logger.debug("Building validation model for %s.", f"{cls.__module__}.{cls.__name__}") - fields = _fields_from_constructor_signature(cls, expand_arbitrary_types=False) + fields: dict[str, tuple[Any, Any]] = _fields_from_constructor_signature(cls, expand_arbitrary_types=False) - model = create_model(f"{cls.__name__}ValidationModel", **fields, __base__=ValidationModel) + model = create_model(f"{cls.__name__}ValidationModel", **cast(Any, fields), __base__=ValidationModel) logger.debug("Created model: %s", model.model_fields) @@ -168,6 +183,11 @@ def __init_subclass__(cls, **kwargs): """ Verify that the subclass defines a validation model. + Parameters + ---------- + **kwargs : Any + Additional keyword arguments passed to ``super().__init_subclass__``. + Raises ------ TypeError diff --git a/tests/common/test_errors.py b/tests/common/test_errors.py index a1960206..bd41bf08 100644 --- a/tests/common/test_errors.py +++ b/tests/common/test_errors.py @@ -14,6 +14,7 @@ def test_tune(install_test_package): with pytest.raises(PyAMLConfigException) as exc: ml: Accelerator = Accelerator.load("tests/config/bad_conf_duplicate_1.yaml", include_locations=True, validate=True) + print(exc.value) assert "MagnetArray HCORR : duplicate name SH1A-C02-H @index 2" in str(exc.value) with pytest.raises(PyAMLConfigException) as exc: diff --git a/tests/configuration/test_factory.py b/tests/configuration/test_factory.py index e4ac88f0..692706bc 100644 --- a/tests/configuration/test_factory.py +++ b/tests/configuration/test_factory.py @@ -25,6 +25,6 @@ def test_factory_build_default(): ) def test_error_cycles(test_file): with pytest.raises(PyAMLException) as exc: - ml: Accelerator = Accelerator.load(test_file, include_locations=True) + ml: Accelerator = Accelerator.load(test_file) assert "Circular file inclusion of " in str(exc.value) diff --git a/tests/validation/test_models.py b/tests/validation/test_models.py index 0c362716..2ec058c6 100644 --- a/tests/validation/test_models.py +++ b/tests/validation/test_models.py @@ -6,6 +6,7 @@ from pydantic import BaseModel, ValidationError from pydantic.errors import PydanticSchemaGenerationError +from pyaml.common.exception import PyAMLConfigException from pyaml.validation import ConfigurationSchema, DynamicValidation, StaticValidation from pyaml.validation.configuration_models import PyAMLBaseModel from pyaml.validation.validation_models import ValidationModel @@ -259,7 +260,7 @@ def __init__(self, name: str, count: int): obj = MyClass(name="test", count="12") assert obj.count == 12 - with pytest.raises(ValidationError): + with pytest.raises(PyAMLConfigException): MyClass(name="test", count="not-an-int") @@ -334,7 +335,7 @@ def __init__(self, name: str, count: int): obj = Example(name="test", count="12") assert obj.count == 12 - with pytest.raises(ValidationError): + with pytest.raises(PyAMLConfigException): Example(name="test", count="not-an-int") diff --git a/tests/validation/test_validation_errors.py b/tests/validation/test_validation_errors.py index f0049d34..4dd3f0c6 100644 --- a/tests/validation/test_validation_errors.py +++ b/tests/validation/test_validation_errors.py @@ -14,7 +14,7 @@ def test_location_str_formats_readably(): loc = Location(file="config.yaml", line=12, column=4) - assert str(loc) == "config.yaml at line 12, column 4." + assert str(loc) == "config.yaml: line 12, column 4" def test_extract_location_metadata_removes_metadata_and_converts_values(): @@ -84,10 +84,10 @@ def test_raise_validation_error_formats_error_with_location_metadata(): message = str(err.value) - assert "'age':" in message + assert "'age'" in message assert "for class: 'pkg.module.Class'" in message - assert "config.yaml at line 21, column 7." in message - assert "config.yaml at line 20, column 1." in message + assert "config.yaml: line 21, column 7" in message + assert "config.yaml: line 20, column 1." in message def test_raise_validation_error_formats_deep_nested_error_tuple_repr(): @@ -100,5 +100,6 @@ def test_raise_validation_error_formats_deep_nested_error_tuple_repr(): ) message = str(err.value) - assert "('items', 0, 'age'):" in message + print(message) + assert "items[0].age" in message assert "for class: 'demo.DeepNestedModel'" in message diff --git a/tests/validation/test_validator.py b/tests/validation/test_validator.py index 5a937d29..3f357209 100644 --- a/tests/validation/test_validator.py +++ b/tests/validation/test_validator.py @@ -258,6 +258,6 @@ def test_recursive_validate_includes_location_metadata_in_error( message = str(exc_info.value) assert "pkg.module.Class" in message - assert "config.yaml at line 10, column 4." in message - assert "config.yaml at line 11, column 8." in message + assert "config.yaml: line 10, column 4." in message + assert "config.yaml: line 11, column 8" in message assert "'value'" in message