Merge remote-tracking branch 'upstream/main' into vega-versioning

vega · Jan 2, 2025 · 88ad05a · 88ad05a
2 parents d49f0ff + b6253ff
commit 88ad05a
Show file tree

Hide file tree

Showing 9 changed files with 176 additions and 109 deletions.
diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py
@@ -7,6 +7,7 @@
 import datetime as dt
 import inspect
 import json
+import operator
 import sys
 import textwrap
 from collections import defaultdict
@@ -48,6 +49,7 @@
     from types import ModuleType
     from typing import ClassVar
 
+    from jsonschema.exceptions import ValidationError
     from referencing import Registry
 
     from altair.typing import ChartType
@@ -589,7 +591,23 @@ def _resolve_references(
     return schema
 
 
+def _validator_values(errors: Iterable[ValidationError], /) -> Iterator[str]:
+    """Unwrap each error's ``.validator_value``, convince ``mypy`` it stores a string."""
+    for err in errors:
+        yield cast("str", err.validator_value)
+
+
 class SchemaValidationError(jsonschema.ValidationError):
+    _JS_TO_PY: ClassVar[Mapping[str, str]] = {
+        "boolean": "bool",
+        "integer": "int",
+        "number": "float",
+        "string": "str",
+        "null": "None",
+        "object": "Mapping[str, Any]",
+        "array": "Sequence",
+    }
+
     def __init__(self, obj: SchemaBase, err: jsonschema.ValidationError) -> None:
         """
         A wrapper for ``jsonschema.ValidationError`` with friendlier traceback.
@@ -762,26 +780,39 @@ def split_into_equal_parts(n: int, p: int) -> list[int]:
                     param_names_table += "\n"
         return param_names_table
 
+    def _format_type_reprs(self, errors: Iterable[ValidationError], /) -> str:
+        """
+        Translate jsonschema types to how they appear in annotations.
+
+        Adapts parts of:
+        - `tools.schemapi.utils.sort_type_reprs`_
+        - `tools.schemapi.utils.SchemaInfo.to_type_repr`_
+
+        .. _tools.schemapi.utils.sort_type_reprs:
+            https://github.com/vega/altair/blob/48e976ef9388ce08a2e871a0f67ed012b914597a/tools/schemapi/utils.py#L1106-L1146
+        .. _tools.schemapi.utils.SchemaInfo.to_type_repr:
+            https://github.com/vega/altair/blob/48e976ef9388ce08a2e871a0f67ed012b914597a/tools/schemapi/utils.py#L449-L543
+        """
+        to_py_types = (
+            self._JS_TO_PY.get(val, val) for val in _validator_values(errors)
+        )
+        it = sorted(to_py_types, key=str.lower)
+        it = sorted(it, key=len)
+        it = sorted(it, key=partial(operator.eq, "None"))
+        return f"of type `{' | '.join(it)}`"
+
     def _get_default_error_message(
         self,
         errors: ValidationErrorList,
     ) -> str:
         bullet_points: list[str] = []
         errors_by_validator = _group_errors_by_validator(errors)
-        if "enum" in errors_by_validator:
-            for error in errors_by_validator["enum"]:
-                bullet_points.append(f"one of {error.validator_value}")
-
-        if "type" in errors_by_validator:
-            types = [f"'{err.validator_value}'" for err in errors_by_validator["type"]]
-            point = "of type "
-            if len(types) == 1:
-                point += types[0]
-            elif len(types) == 2:
-                point += f"{types[0]} or {types[1]}"
-            else:
-                point += ", ".join(types[:-1]) + f", or {types[-1]}"
-            bullet_points.append(point)
+        if errs_enum := errors_by_validator.get("enum", None):
+            bullet_points.extend(
+                f"one of {val}" for val in _validator_values(errs_enum)
+            )
+        if errs_type := errors_by_validator.get("type", None):
+            bullet_points.append(self._format_type_reprs(errs_type))
 
         # It should not matter which error is specifically used as they are all
         # about the same offending instance (i.e. invalid value), so we can just

diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py
@@ -3939,11 +3939,8 @@ def __init__(
         height: Optional[int | dict | Step | Literal["container"]] = Undefined,
         **kwargs: Any,
     ) -> None:
-        # Data type hints won't match with what TopLevelUnitSpec expects
-        # as there is some data processing happening when converting to
-        # a VL spec
         super().__init__(
-            data=data,  # type: ignore[arg-type]
+            data=data,
             encoding=encoding,
             mark=mark,
             width=width,
@@ -4328,7 +4325,7 @@ def __init__(
     ) -> None:
         for spec in concat:
             _check_if_valid_subspec(spec, "ConcatChart")
-        super().__init__(data=data, concat=list(concat), columns=columns, **kwargs)  # type: ignore[arg-type]
+        super().__init__(data=data, concat=list(concat), columns=columns, **kwargs)
         self.concat: list[ChartType]
         self.params: Optional[Sequence[_Parameter]]
         self.data: Optional[ChartDataType]
@@ -4432,7 +4429,7 @@ def __init__(
     ) -> None:
         for spec in hconcat:
             _check_if_valid_subspec(spec, "HConcatChart")
-        super().__init__(data=data, hconcat=list(hconcat), **kwargs)  # type: ignore[arg-type]
+        super().__init__(data=data, hconcat=list(hconcat), **kwargs)
         self.hconcat: list[ChartType]
         self.params: Optional[Sequence[_Parameter]]
         self.data: Optional[ChartDataType]
@@ -4536,7 +4533,7 @@ def __init__(
     ) -> None:
         for spec in vconcat:
             _check_if_valid_subspec(spec, "VConcatChart")
-        super().__init__(data=data, vconcat=list(vconcat), **kwargs)  # type: ignore[arg-type]
+        super().__init__(data=data, vconcat=list(vconcat), **kwargs)
         self.vconcat: list[ChartType]
         self.params: Optional[Sequence[_Parameter]]
         self.data: Optional[ChartDataType]
@@ -4644,7 +4641,7 @@ def __init__(
         for spec in layer:
             _check_if_valid_subspec(spec, "LayerChart")
             _check_if_can_be_layered(spec)
-        super().__init__(data=data, layer=list(layer), **kwargs)  # type: ignore[arg-type]
+        super().__init__(data=data, layer=list(layer), **kwargs)
         self.layer: list[ChartType]
         self.params: Optional[Sequence[_Parameter]]
         self.data: Optional[ChartDataType]
@@ -4775,7 +4772,7 @@ def __init__(
         _spec_as_list = [spec]
         params, _spec_as_list = _combine_subchart_params(params, _spec_as_list)
         spec = _spec_as_list[0]
-        super().__init__(data=data, spec=spec, facet=facet, params=params, **kwargs)  # type: ignore[arg-type]
+        super().__init__(data=data, spec=spec, facet=facet, params=params, **kwargs)
         self.data: Optional[ChartDataType]
         self.spec: ChartType
         self.params: Optional[Sequence[_Parameter]]

diff --git a/altair/vegalite/v5/schema/core.py b/altair/vegalite/v5/schema/core.py
@@ -20,6 +20,7 @@
 
     from altair import Parameter
     from altair.typing import Optional
+    from altair.vegalite.v5.api import ChartDataType
 
     from ._typing import *  # noqa: F403
 
@@ -7887,7 +7888,7 @@ class GenericUnitSpecEncodingAnyMark(VegaLiteSchema):
     def __init__(
         self,
         mark: Optional[SchemaBase | Map | Mark_T | CompositeMark_T] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         encoding: Optional[SchemaBase | Map] = Undefined,
         name: Optional[str] = Undefined,
@@ -11062,7 +11063,7 @@ class LookupData(VegaLiteSchema):
 
     def __init__(
         self,
-        data: Optional[SchemaBase | Map] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map] = Undefined,
         key: Optional[str | SchemaBase] = Undefined,
         fields: Optional[Sequence[str | SchemaBase]] = Undefined,
         **kwds,
@@ -21055,7 +21056,7 @@ def __init__(
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool | SchemaBase | Map] = Undefined,
         columns: Optional[float] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
         resolve: Optional[SchemaBase | Map] = Undefined,
@@ -21182,7 +21183,7 @@ def __init__(
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool | SchemaBase | Map] = Undefined,
         columns: Optional[float] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
         resolve: Optional[SchemaBase | Map] = Undefined,
@@ -21340,7 +21341,7 @@ def __init__(
         align: Optional[SchemaBase | Map | LayoutAlign_T] = Undefined,
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool | SchemaBase | Map] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         encoding: Optional[SchemaBase | Map] = Undefined,
         height: Optional[float | SchemaBase | Literal["container"] | Map] = Undefined,
@@ -21429,7 +21430,7 @@ def __init__(
         hconcat: Optional[Sequence[SchemaBase | Map]] = Undefined,
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
         resolve: Optional[SchemaBase | Map] = Undefined,
@@ -21537,7 +21538,7 @@ class LayerSpec(Spec, NonNormalizedSpec):
     def __init__(
         self,
         layer: Optional[Sequence[SchemaBase | Map]] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         encoding: Optional[SchemaBase | Map] = Undefined,
         height: Optional[float | SchemaBase | Literal["container"] | Map] = Undefined,
@@ -21677,7 +21678,7 @@ def __init__(
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool | SchemaBase | Map] = Undefined,
         columns: Optional[float] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
         resolve: Optional[SchemaBase | Map] = Undefined,
@@ -21807,7 +21808,7 @@ def __init__(
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool | SchemaBase | Map] = Undefined,
         columns: Optional[float] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
         resolve: Optional[SchemaBase | Map] = Undefined,
@@ -24400,7 +24401,7 @@ def __init__(
         center: Optional[bool | SchemaBase | Map] = Undefined,
         columns: Optional[float] = Undefined,
         config: Optional[SchemaBase | Map] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         datasets: Optional[SchemaBase | Map] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
@@ -24565,7 +24566,7 @@ class TopLevelFacetSpec(TopLevelSpec):
 
     def __init__(
         self,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         facet: Optional[SchemaBase | Map] = Undefined,
         spec: Optional[SchemaBase | Map] = Undefined,
         align: Optional[SchemaBase | Map | LayoutAlign_T] = Undefined,
@@ -24704,7 +24705,7 @@ def __init__(
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool] = Undefined,
         config: Optional[SchemaBase | Map] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         datasets: Optional[SchemaBase | Map] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
@@ -24860,7 +24861,7 @@ def __init__(
             str | Parameter | SchemaBase | Map | ColorName_T
         ] = Undefined,
         config: Optional[SchemaBase | Map] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         datasets: Optional[SchemaBase | Map] = Undefined,
         description: Optional[str] = Undefined,
         encoding: Optional[SchemaBase | Map] = Undefined,
@@ -25067,7 +25068,7 @@ class TopLevelUnitSpec(TopLevelSpec):
 
     def __init__(
         self,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         mark: Optional[SchemaBase | Map | Mark_T | CompositeMark_T] = Undefined,
         align: Optional[SchemaBase | Map | LayoutAlign_T] = Undefined,
         autosize: Optional[SchemaBase | Map | AutosizeType_T] = Undefined,
@@ -25212,7 +25213,7 @@ def __init__(
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool] = Undefined,
         config: Optional[SchemaBase | Map] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         datasets: Optional[SchemaBase | Map] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
@@ -26229,7 +26230,7 @@ class UnitSpec(VegaLiteSchema):
     def __init__(
         self,
         mark: Optional[SchemaBase | Map | Mark_T | CompositeMark_T] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         encoding: Optional[SchemaBase | Map] = Undefined,
         name: Optional[str] = Undefined,
@@ -26333,7 +26334,7 @@ class UnitSpecWithFrame(VegaLiteSchema):
     def __init__(
         self,
         mark: Optional[SchemaBase | Map | Mark_T | CompositeMark_T] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         encoding: Optional[SchemaBase | Map] = Undefined,
         height: Optional[float | SchemaBase | Literal["container"] | Map] = Undefined,
@@ -26460,7 +26461,7 @@ def __init__(
         vconcat: Optional[Sequence[SchemaBase | Map]] = Undefined,
         bounds: Optional[Literal["full", "flush"]] = Undefined,
         center: Optional[bool] = Undefined,
-        data: Optional[SchemaBase | Map | None] = Undefined,
+        data: Optional[SchemaBase | ChartDataType | Map | None] = Undefined,
         description: Optional[str] = Undefined,
         name: Optional[str] = Undefined,
         resolve: Optional[SchemaBase | Map] = Undefined,