Improve validation errors

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
 

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,25 +13,35 @@ 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
     line: int
     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)
 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 ``<root>`` 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 "<root>"
+
+
 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

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

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:

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)

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")