diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 3d81abd9f..f050dd1d1 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3844,7 +3844,7 @@ def interactive( def _check_if_valid_subspec( - spec: ConcatType | LayerType | dict[str, Any], + spec: ConcatType | LayerType, classname: Literal[ "ConcatChart", "FacetChart", @@ -3854,65 +3854,50 @@ def _check_if_valid_subspec( "VConcatChart", ], ) -> None: - """ - Check if the spec is a valid sub-spec. - - If it is not, then raise a ValueError - """ - err = ( - 'Objects with "{0}" attribute cannot be used within {1}. ' - "Consider defining the {0} attribute in the {1} object instead." - ) - - if not isinstance(spec, (core.SchemaBase, dict)): + """Raise a `TypeError` if `spec` is not a valid sub-spec.""" + if not isinstance(spec, core.SchemaBase): msg = f"Only chart objects can be used in {classname}." - raise ValueError(msg) + raise TypeError(msg) for attr in TOPLEVEL_ONLY_KEYS: - if isinstance(spec, core.SchemaBase): - val = getattr(spec, attr, Undefined) - else: - val = spec.get(attr, Undefined) - if val is not Undefined: - raise ValueError(err.format(attr, classname)) - + if spec._get(attr) is not Undefined: + msg = ( + f"Objects with {attr!r} attribute cannot be used within {classname}. " + f"Consider defining the {attr} attribute in the {classname} object instead." + ) + raise TypeError(msg) -# C901 fixed in https://github.com/vega/altair/pull/3520 -def _check_if_can_be_layered(spec: LayerType | dict[str, Any]) -> None: # noqa: C901 - """Check if the spec can be layered.""" - def _get(spec: LayerType | dict[str, Any], attr: str) -> Any: - if isinstance(spec, core.SchemaBase): - return spec._get(attr) - else: - return spec.get(attr, Undefined) +def _check_if_can_be_layered(spec: LayerType) -> None: + """Raise a `TypeError` if `spec` cannot be layered.""" - def _get_any(spec: LayerType | dict[str, Any], *attrs: str) -> bool: - return any(_get(spec, attr) is not Undefined for attr in attrs) + def _get_any(spec: LayerType, *attrs: str) -> bool: + return any(spec._get(attr) is not Undefined for attr in attrs) base_msg = "charts cannot be layered. Instead, layer the charts before" - encoding = _get(spec, "encoding") - if encoding is not Undefined: + encoding: Any = spec._get("encoding") + if not utils.is_undefined(encoding): for channel in ["row", "column", "facet"]: - if _get(encoding, channel) is not Undefined: + if encoding._get(channel) is not Undefined: msg = f"Faceted {base_msg} faceting." - raise ValueError(msg) + raise TypeError(msg) if isinstance(spec, (Chart, LayerChart)): return - - if not isinstance(spec, (core.SchemaBase, dict)): - msg = "Only chart objects can be layered." - raise ValueError(msg) - if isinstance(spec, FacetChart) or _get(spec, "facet") is not Undefined: - msg = f"Faceted {base_msg} faceting." - raise ValueError(msg) - if isinstance(spec, RepeatChart) or _get(spec, "repeat") is not Undefined: - msg = f"Repeat {base_msg} repeating." - raise ValueError(msg) - _concat = ConcatChart, HConcatChart, VConcatChart - if isinstance(spec, _concat) or _get_any(spec, "concat", "hconcat", "vconcat"): - msg = f"Concatenated {base_msg} concatenating." - raise ValueError(msg) + elif is_chart_type(spec) or _get_any( + spec, "facet", "repeat", "concat", "hconcat", "vconcat" + ): + if isinstance(spec, FacetChart) or spec._get("facet") is not Undefined: + msg = f"Faceted {base_msg} faceting." + elif isinstance(spec, RepeatChart) or spec._get("repeat") is not Undefined: + msg = f"Repeat {base_msg} repeating." + elif isinstance(spec, (ConcatChart, HConcatChart, VConcatChart)) or _get_any( + spec, "concat", "hconcat", "vconcat" + ): + msg = f"Concatenated {base_msg} concatenating." + else: + msg = "Should be unreachable" + raise NotImplementedError(msg) + raise TypeError(msg) class RepeatChart(TopLevelMixin, core.TopLevelRepeatSpec): diff --git a/pyproject.toml b/pyproject.toml index 0eba4cefe..e2bdf7783 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -434,3 +434,4 @@ ignore_missing_imports = true extraPaths=["./tools"] pythonPlatform="All" pythonVersion="3.8" +reportUnusedExpression="none" diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index 9642ec013..167b71ed5 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1403,39 +1403,20 @@ def test_layer_errors(): simple_chart = alt.Chart("data.txt").mark_point() - with pytest.raises(ValueError) as err: # noqa: PT011 + with pytest.raises(TypeError, match=r".config. attribute cannot.+LayerChart"): toplevel_chart + simple_chart - assert str(err.value).startswith( - 'Objects with "config" attribute cannot be used within LayerChart.' - ) - with pytest.raises(ValueError) as err: # noqa: PT011 + with pytest.raises(TypeError, match=r"Concat.+cannot.+layered.+before concat"): alt.hconcat(simple_chart) + simple_chart - assert ( - str(err.value) - == "Concatenated charts cannot be layered. Instead, layer the charts before concatenating." - ) - with pytest.raises(ValueError) as err: # noqa: PT011 + with pytest.raises(TypeError, match=r"Repeat.+cannot.+layered.+before repeat"): repeat_chart + simple_chart - assert ( - str(err.value) - == "Repeat charts cannot be layered. Instead, layer the charts before repeating." - ) - with pytest.raises(ValueError) as err: # noqa: PT011 + with pytest.raises(TypeError, match=r"Facet.+.+cannot.+layered.+before facet"): facet_chart1 + simple_chart - assert ( - str(err.value) - == "Faceted charts cannot be layered. Instead, layer the charts before faceting." - ) - with pytest.raises(ValueError) as err: # noqa: PT011 + with pytest.raises(TypeError, match=r"Facet.+.+cannot.+layered.+before facet"): alt.layer(simple_chart) + facet_chart2 - assert ( - str(err.value) - == "Faceted charts cannot be layered. Instead, layer the charts before faceting." - ) @pytest.mark.parametrize(