From 8aa59d834e023cee1495abcb921851dd39553128 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 16:50:25 +0100 Subject: [PATCH 01/18] refactor: Move comment to `SchemaValidationError.__init__` No information is lost, still available on hover. The comment itself is not helpful to users and would be presented on every validation error --- altair/utils/schemapi.py | 25 ++++++++++++++++++------- tools/schemapi/schemapi.py | 25 ++++++++++++++++++------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index de196025b..abe42db4d 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -543,9 +543,25 @@ def _resolve_references( class SchemaValidationError(jsonschema.ValidationError): - """A wrapper for jsonschema.ValidationError with friendlier traceback.""" - def __init__(self, obj: SchemaBase, err: jsonschema.ValidationError) -> None: + """ + A wrapper for ``jsonschema.ValidationError`` with friendlier traceback. + + Parameters + ---------- + obj + The instance that failed ``self.validate(...)``. + err + The original ``ValidationError``. + + Notes + ----- + We do not raise `from err` as else the resulting traceback is very long + as it contains part of the Vega-Lite schema. + + It would also first show the less helpful `ValidationError` instead of + the more user friendly `SchemaValidationError`. + """ super().__init__(**err._contents()) self.obj = obj self._errors: GroupedValidationErrors = getattr( @@ -1055,11 +1071,6 @@ def to_dict( try: self.validate(result) except jsonschema.ValidationError as err: - # We do not raise `from err` as else the resulting - # traceback is very long as it contains part - # of the Vega-Lite schema. It would also first - # show the less helpful ValidationError instead of - # the more user friendly SchemaValidationError raise SchemaValidationError(self, err) from None return result diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index 0ef56ccf7..ad15bb0af 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -541,9 +541,25 @@ def _resolve_references( class SchemaValidationError(jsonschema.ValidationError): - """A wrapper for jsonschema.ValidationError with friendlier traceback.""" - def __init__(self, obj: SchemaBase, err: jsonschema.ValidationError) -> None: + """ + A wrapper for ``jsonschema.ValidationError`` with friendlier traceback. + + Parameters + ---------- + obj + The instance that failed ``self.validate(...)``. + err + The original ``ValidationError``. + + Notes + ----- + We do not raise `from err` as else the resulting traceback is very long + as it contains part of the Vega-Lite schema. + + It would also first show the less helpful `ValidationError` instead of + the more user friendly `SchemaValidationError`. + """ super().__init__(**err._contents()) self.obj = obj self._errors: GroupedValidationErrors = getattr( @@ -1053,11 +1069,6 @@ def to_dict( try: self.validate(result) except jsonschema.ValidationError as err: - # We do not raise `from err` as else the resulting - # traceback is very long as it contains part - # of the Vega-Lite schema. It would also first - # show the less helpful ValidationError instead of - # the more user friendly SchemaValidationError raise SchemaValidationError(self, err) from None return result From 1f608473319783518ef083802d638629778f91ac Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 17:17:14 +0100 Subject: [PATCH 02/18] docs: Reduce repetition and rearrange `SchemaBase.to_dict` Moved repeat info to *Notes*, which should come last https://numpydoc.readthedocs.io/en/latest/format.html#notes Removed redundant *Returns* Minor changes to improve readability --- altair/utils/schemapi.py | 26 +++++++++----------------- tools/schemapi/schemapi.py | 26 +++++++++----------------- 2 files changed, 18 insertions(+), 34 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index abe42db4d..907469094 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -994,29 +994,21 @@ def to_dict( Parameters ---------- validate : bool, optional - If True (default), then validate the output dictionary - against the schema. + If True (default), then validate the result against the schema. ignore : list[str], optional - A list of keys to ignore. It is usually not needed - to specify this argument as a user. + A list of keys to ignore. context : dict[str, Any], optional - A context dictionary. It is usually not needed - to specify this argument as a user. - - Notes - ----- - Technical: The ignore parameter will *not* be passed to child to_dict - function calls. - - Returns - ------- - dict - The dictionary representation of this object + A context dictionary. Raises ------ SchemaValidationError : - if validate=True and the dict does not conform to the schema + If ``validate`` and the result does not conform to the schema. + + Notes + ----- + - ``ignore``, ``context`` are usually not needed to be specified as a user. + - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ if context is None: context = {} diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index ad15bb0af..d91d03f94 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -992,29 +992,21 @@ def to_dict( Parameters ---------- validate : bool, optional - If True (default), then validate the output dictionary - against the schema. + If True (default), then validate the result against the schema. ignore : list[str], optional - A list of keys to ignore. It is usually not needed - to specify this argument as a user. + A list of keys to ignore. context : dict[str, Any], optional - A context dictionary. It is usually not needed - to specify this argument as a user. - - Notes - ----- - Technical: The ignore parameter will *not* be passed to child to_dict - function calls. - - Returns - ------- - dict - The dictionary representation of this object + A context dictionary. Raises ------ SchemaValidationError : - if validate=True and the dict does not conform to the schema + If ``validate`` and the result does not conform to the schema. + + Notes + ----- + - ``ignore``, ``context`` are usually not needed to be specified as a user. + - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ if context is None: context = {} From 90b0a03df6b9bcb4783a86409f78e4dbd68f5ce7 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 17:19:05 +0100 Subject: [PATCH 03/18] style: Use a single line for single line error message --- altair/utils/schemapi.py | 5 +---- tools/schemapi/schemapi.py | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 907469094..9846d26b7 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1054,10 +1054,7 @@ def to_dict( kwds["mark"] = {"type": kwds["mark"]} result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) else: - msg = ( - f"{self.__class__} instance has both a value and properties : " - "cannot serialize to dict" - ) + msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" raise ValueError(msg) if validate: try: diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index d91d03f94..cb088514e 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1052,10 +1052,7 @@ def to_dict( kwds["mark"] = {"type": kwds["mark"]} result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) else: - msg = ( - f"{self.__class__} instance has both a value and properties : " - "cannot serialize to dict" - ) + msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" raise ValueError(msg) if validate: try: From f700cffb6efdb312cf7e7eb187105faa432ade0c Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 18:18:19 +0100 Subject: [PATCH 04/18] refactor: Use `genexpr` instead of `dictcomp` Saves 2 lines, identical result, maybe faster due to not allocating an intermediate dict --- altair/utils/schemapi.py | 8 +++----- tools/schemapi/schemapi.py | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 9846d26b7..440d85c29 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1041,11 +1041,9 @@ def to_dict( parsed_shorthand.pop("sort") kwds.update( - { - k: v - for k, v in parsed_shorthand.items() - if kwds.get(k, Undefined) is Undefined - } + (k, v) + for k, v in parsed_shorthand.items() + if kwds.get(k, Undefined) is Undefined ) kwds = { k: v for k, v in kwds.items() if k not in {*list(ignore), "shorthand"} diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index cb088514e..ca71554ec 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1039,11 +1039,9 @@ def to_dict( parsed_shorthand.pop("sort") kwds.update( - { - k: v - for k, v in parsed_shorthand.items() - if kwds.get(k, Undefined) is Undefined - } + (k, v) + for k, v in parsed_shorthand.items() + if kwds.get(k, Undefined) is Undefined ) kwds = { k: v for k, v in kwds.items() if k not in {*list(ignore), "shorthand"} From 0c2e21610da24a866a50c052c8a6fe9aef4927c4 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 18:28:01 +0100 Subject: [PATCH 05/18] refactor: Use ternary exprs for `context`, `ignore` All `...ChannelMixin` already use this form. Saves 2 lines --- altair/utils/schemapi.py | 6 ++---- tools/schemapi/schemapi.py | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 440d85c29..62a7d073e 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1010,10 +1010,8 @@ def to_dict( - ``ignore``, ``context`` are usually not needed to be specified as a user. - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ - if context is None: - context = {} - if ignore is None: - ignore = [] + context = context or {} + ignore = ignore or [] # The following return the package only if it has already been # imported - otherwise they return None. This is useful for # isinstance checks - for example, if pandas has not been imported, diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index ca71554ec..a135fb864 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1008,10 +1008,8 @@ def to_dict( - ``ignore``, ``context`` are usually not needed to be specified as a user. - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ - if context is None: - context = {} - if ignore is None: - ignore = [] + context = context or {} + ignore = ignore or [] # The following return the package only if it has already been # imported - otherwise they return None. This is useful for # isinstance checks - for example, if pandas has not been imported, From 7a121df8c83cc086aa5241cc187fd4120075588a Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 18:51:22 +0100 Subject: [PATCH 06/18] refactor: Alias `kwds` filter to `exclude` Saves only 1 line, but collapses the dict comp. The `:=` expr doesn't reduce lines, but does reduce characters --- altair/utils/schemapi.py | 9 ++++----- tools/schemapi/schemapi.py | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 62a7d073e..92cba8f01 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1025,6 +1025,7 @@ def to_dict( ) elif not self._args: kwds = self._kwds.copy() + exclude = {*ignore, "shorthand"} # parsed_shorthand is added by FieldChannelMixin. # It's used below to replace shorthand with its long form equivalent # parsed_shorthand is removed from context if it exists so that it is @@ -1043,11 +1044,9 @@ def to_dict( for k, v in parsed_shorthand.items() if kwds.get(k, Undefined) is Undefined ) - kwds = { - k: v for k, v in kwds.items() if k not in {*list(ignore), "shorthand"} - } - if "mark" in kwds and isinstance(kwds["mark"], str): - kwds["mark"] = {"type": kwds["mark"]} + kwds = {k: v for k, v in kwds.items() if k not in exclude} + if (mark := kwds.get("mark")) and isinstance(mark, str): + kwds["mark"] = {"type": mark} result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) else: msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index a135fb864..dc7dfec9e 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1023,6 +1023,7 @@ def to_dict( ) elif not self._args: kwds = self._kwds.copy() + exclude = {*ignore, "shorthand"} # parsed_shorthand is added by FieldChannelMixin. # It's used below to replace shorthand with its long form equivalent # parsed_shorthand is removed from context if it exists so that it is @@ -1041,11 +1042,9 @@ def to_dict( for k, v in parsed_shorthand.items() if kwds.get(k, Undefined) is Undefined ) - kwds = { - k: v for k, v in kwds.items() if k not in {*list(ignore), "shorthand"} - } - if "mark" in kwds and isinstance(kwds["mark"], str): - kwds["mark"] = {"type": kwds["mark"]} + kwds = {k: v for k, v in kwds.items() if k not in exclude} + if (mark := kwds.get("mark")) and isinstance(mark, str): + kwds["mark"] = {"type": mark} result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) else: msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" From 502140becaed827232476d3a6fc035a5080a1c74 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 19:04:27 +0100 Subject: [PATCH 07/18] refactor: Call `_to_dict` from a single site Saves 2 lines, avoids repeating the 3 kwargs - but allows modified `context` --- altair/utils/schemapi.py | 6 ++---- tools/schemapi/schemapi.py | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 92cba8f01..18b217cb8 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1020,9 +1020,7 @@ def to_dict( np_opt = sys.modules.get("numpy") if self._args and not self._kwds: - result = _todict( - self._args[0], context=context, np_opt=np_opt, pd_opt=pd_opt - ) + kwds = self._args[0] elif not self._args: kwds = self._kwds.copy() exclude = {*ignore, "shorthand"} @@ -1047,10 +1045,10 @@ def to_dict( kwds = {k: v for k, v in kwds.items() if k not in exclude} if (mark := kwds.get("mark")) and isinstance(mark, str): kwds["mark"] = {"type": mark} - result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) else: msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" raise ValueError(msg) + result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) if validate: try: self.validate(result) diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index dc7dfec9e..8dc4bd804 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1018,9 +1018,7 @@ def to_dict( np_opt = sys.modules.get("numpy") if self._args and not self._kwds: - result = _todict( - self._args[0], context=context, np_opt=np_opt, pd_opt=pd_opt - ) + kwds = self._args[0] elif not self._args: kwds = self._kwds.copy() exclude = {*ignore, "shorthand"} @@ -1045,10 +1043,10 @@ def to_dict( kwds = {k: v for k, v in kwds.items() if k not in exclude} if (mark := kwds.get("mark")) and isinstance(mark, str): kwds["mark"] = {"type": mark} - result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) else: msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" raise ValueError(msg) + result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) if validate: try: self.validate(result) From fca2e625662aa7e352219cba69b580a5fd783cf7 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 19:26:08 +0100 Subject: [PATCH 08/18] wip --- tests/utils/test_schemapi.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/utils/test_schemapi.py b/tests/utils/test_schemapi.py index b0068724b..ea45242f8 100644 --- a/tests/utils/test_schemapi.py +++ b/tests/utils/test_schemapi.py @@ -496,6 +496,11 @@ def chart_error_example__invalid_bandposition_value(): ) +def test_to_dict_huge_traceback(): + # Error: Invalid value for type + return alt.Chart().encode(alt.X(type="unknown")).to_dict() + + def chart_error_example__invalid_type(): # Error: Invalid value for type return alt.Chart().encode(alt.X(type="unknown")) From 4943c34435102fe57f7957e11442fa11c7ded222 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Thu, 8 Aug 2024 20:39:44 +0100 Subject: [PATCH 09/18] refactor: Factor out large block to `_replace_parsed_shorthand` As with the change to `SchemaValidationError`, the information is available in a docstring if needed. But no longer is displayed to the user on a validation error --- altair/utils/schemapi.py | 46 +++++++++++++++++++++++--------------- tools/schemapi/schemapi.py | 46 +++++++++++++++++++++++--------------- 2 files changed, 56 insertions(+), 36 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 18b217cb8..607274688 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1024,24 +1024,8 @@ def to_dict( elif not self._args: kwds = self._kwds.copy() exclude = {*ignore, "shorthand"} - # parsed_shorthand is added by FieldChannelMixin. - # It's used below to replace shorthand with its long form equivalent - # parsed_shorthand is removed from context if it exists so that it is - # not passed to child to_dict function calls - parsed_shorthand = context.pop("parsed_shorthand", {}) - # Prevent that pandas categorical data is automatically sorted - # when a non-ordinal data type is specifed manually - # or if the encoding channel does not support sorting - if "sort" in parsed_shorthand and ( - "sort" not in kwds or kwds["type"] not in {"ordinal", Undefined} - ): - parsed_shorthand.pop("sort") - - kwds.update( - (k, v) - for k, v in parsed_shorthand.items() - if kwds.get(k, Undefined) is Undefined - ) + if parsed := context.pop("parsed_shorthand", None): + kwds = _replace_parsed_shorthand(parsed, kwds) kwds = {k: v for k, v in kwds.items() if k not in exclude} if (mark := kwds.get("mark")) and isinstance(mark, str): kwds["mark"] = {"type": mark} @@ -1212,6 +1196,32 @@ def __dir__(self) -> list[str]: return sorted(chain(super().__dir__(), self._kwds)) +def _replace_parsed_shorthand( + parsed_shorthand: dict[str, Any], kwds: dict[str, Any] +) -> dict[str, Any]: + """ + `parsed_shorthand` is added by `FieldChannelMixin`. + + It's used below to replace shorthand with its long form equivalent + `parsed_shorthand` is removed from `context` if it exists so that it is + not passed to child `to_dict` function calls. + """ + # Prevent that pandas categorical data is automatically sorted + # when a non-ordinal data type is specifed manually + # or if the encoding channel does not support sorting + if "sort" in parsed_shorthand and ( + "sort" not in kwds or kwds["type"] not in {"ordinal", Undefined} + ): + parsed_shorthand.pop("sort") + + kwds.update( + (k, v) + for k, v in parsed_shorthand.items() + if kwds.get(k, Undefined) is Undefined + ) + return kwds + + TSchemaBase = TypeVar("TSchemaBase", bound=SchemaBase) diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index 8dc4bd804..96a870a1b 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1022,24 +1022,8 @@ def to_dict( elif not self._args: kwds = self._kwds.copy() exclude = {*ignore, "shorthand"} - # parsed_shorthand is added by FieldChannelMixin. - # It's used below to replace shorthand with its long form equivalent - # parsed_shorthand is removed from context if it exists so that it is - # not passed to child to_dict function calls - parsed_shorthand = context.pop("parsed_shorthand", {}) - # Prevent that pandas categorical data is automatically sorted - # when a non-ordinal data type is specifed manually - # or if the encoding channel does not support sorting - if "sort" in parsed_shorthand and ( - "sort" not in kwds or kwds["type"] not in {"ordinal", Undefined} - ): - parsed_shorthand.pop("sort") - - kwds.update( - (k, v) - for k, v in parsed_shorthand.items() - if kwds.get(k, Undefined) is Undefined - ) + if parsed := context.pop("parsed_shorthand", None): + kwds = _replace_parsed_shorthand(parsed, kwds) kwds = {k: v for k, v in kwds.items() if k not in exclude} if (mark := kwds.get("mark")) and isinstance(mark, str): kwds["mark"] = {"type": mark} @@ -1210,6 +1194,32 @@ def __dir__(self) -> list[str]: return sorted(chain(super().__dir__(), self._kwds)) +def _replace_parsed_shorthand( + parsed_shorthand: dict[str, Any], kwds: dict[str, Any] +) -> dict[str, Any]: + """ + `parsed_shorthand` is added by `FieldChannelMixin`. + + It's used below to replace shorthand with its long form equivalent + `parsed_shorthand` is removed from `context` if it exists so that it is + not passed to child `to_dict` function calls. + """ + # Prevent that pandas categorical data is automatically sorted + # when a non-ordinal data type is specifed manually + # or if the encoding channel does not support sorting + if "sort" in parsed_shorthand and ( + "sort" not in kwds or kwds["type"] not in {"ordinal", Undefined} + ): + parsed_shorthand.pop("sort") + + kwds.update( + (k, v) + for k, v in parsed_shorthand.items() + if kwds.get(k, Undefined) is Undefined + ) + return kwds + + TSchemaBase = TypeVar("TSchemaBase", bound=SchemaBase) From 44d2873057441d6b5cd66a08e4c91b0e24a9c009 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 9 Aug 2024 09:47:50 +0100 Subject: [PATCH 10/18] refactor: Factor out optional imports to `_get_optional_modules` Removes 5 lines. Has the additional benefit of replacing an identical block in `SchemaBase.validate_property`. Therefore reduces the maintenance burden of keeping comments in sync --- altair/utils/schemapi.py | 43 +++++++++++++++++++++++++------------- tools/schemapi/schemapi.py | 43 +++++++++++++++++++++++++------------- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 607274688..7448974c0 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -40,6 +40,7 @@ from altair import vegalite if TYPE_CHECKING: + from types import ModuleType from typing import ClassVar from referencing import Registry @@ -1012,12 +1013,7 @@ def to_dict( """ context = context or {} ignore = ignore or [] - # The following return the package only if it has already been - # imported - otherwise they return None. This is useful for - # isinstance checks - for example, if pandas has not been imported, - # then an object is definitely not a `pandas.Timestamp`. - pd_opt = sys.modules.get("pandas") - np_opt = sys.modules.get("numpy") + opts = _get_optional_modules(np_opt="numpy", pd_opt="pandas") if self._args and not self._kwds: kwds = self._args[0] @@ -1032,7 +1028,7 @@ def to_dict( else: msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" raise ValueError(msg) - result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) + result = _todict(kwds, context=context, **opts) if validate: try: self.validate(result) @@ -1180,13 +1176,8 @@ def validate_property( cls, name: str, value: Any, schema: dict[str, Any] | None = None ) -> None: """Validate a property against property schema in the context of the rootschema.""" - # The following return the package only if it has already been - # imported - otherwise they return None. This is useful for - # isinstance checks - for example, if pandas has not been imported, - # then an object is definitely not a `pandas.Timestamp`. - pd_opt = sys.modules.get("pandas") - np_opt = sys.modules.get("numpy") - value = _todict(value, context={}, np_opt=np_opt, pd_opt=pd_opt) + opts = _get_optional_modules(np_opt="numpy", pd_opt="pandas") + value = _todict(value, context={}, **opts) props = cls.resolve_references(schema or cls._schema).get("properties", {}) return validate_jsonschema( value, props.get(name, {}), rootschema=cls._rootschema or cls._schema @@ -1196,6 +1187,30 @@ def __dir__(self) -> list[str]: return sorted(chain(super().__dir__(), self._kwds)) +def _get_optional_modules(**modules: str) -> dict[str, ModuleType | None]: + """ + Returns packages only if they have already been imported - otherwise they return `None`. + + This is useful for `isinstance` checks. + + For example, if `pandas` has not been imported, then an object is + definitely not a `pandas.Timestamp`. + + Parameters + ---------- + **modules + Keyword-only binding from `{alias: module_name}`. + + Examples + -------- + :: + + ResultType = dict[Literal["pd", "pl"], ModuleType | None] + r: ResultType = _get_optional_modules(pd="pandas", pl="polars") + """ + return {k: sys.modules.get(v) for k, v in modules.items()} + + def _replace_parsed_shorthand( parsed_shorthand: dict[str, Any], kwds: dict[str, Any] ) -> dict[str, Any]: diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index 96a870a1b..34e24031f 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -38,6 +38,7 @@ from altair import vegalite if TYPE_CHECKING: + from types import ModuleType from typing import ClassVar from referencing import Registry @@ -1010,12 +1011,7 @@ def to_dict( """ context = context or {} ignore = ignore or [] - # The following return the package only if it has already been - # imported - otherwise they return None. This is useful for - # isinstance checks - for example, if pandas has not been imported, - # then an object is definitely not a `pandas.Timestamp`. - pd_opt = sys.modules.get("pandas") - np_opt = sys.modules.get("numpy") + opts = _get_optional_modules(np_opt="numpy", pd_opt="pandas") if self._args and not self._kwds: kwds = self._args[0] @@ -1030,7 +1026,7 @@ def to_dict( else: msg = f"{type(self)} instance has both a value and properties : cannot serialize to dict" raise ValueError(msg) - result = _todict(kwds, context=context, np_opt=np_opt, pd_opt=pd_opt) + result = _todict(kwds, context=context, **opts) if validate: try: self.validate(result) @@ -1178,13 +1174,8 @@ def validate_property( cls, name: str, value: Any, schema: dict[str, Any] | None = None ) -> None: """Validate a property against property schema in the context of the rootschema.""" - # The following return the package only if it has already been - # imported - otherwise they return None. This is useful for - # isinstance checks - for example, if pandas has not been imported, - # then an object is definitely not a `pandas.Timestamp`. - pd_opt = sys.modules.get("pandas") - np_opt = sys.modules.get("numpy") - value = _todict(value, context={}, np_opt=np_opt, pd_opt=pd_opt) + opts = _get_optional_modules(np_opt="numpy", pd_opt="pandas") + value = _todict(value, context={}, **opts) props = cls.resolve_references(schema or cls._schema).get("properties", {}) return validate_jsonschema( value, props.get(name, {}), rootschema=cls._rootschema or cls._schema @@ -1194,6 +1185,30 @@ def __dir__(self) -> list[str]: return sorted(chain(super().__dir__(), self._kwds)) +def _get_optional_modules(**modules: str) -> dict[str, ModuleType | None]: + """ + Returns packages only if they have already been imported - otherwise they return `None`. + + This is useful for `isinstance` checks. + + For example, if `pandas` has not been imported, then an object is + definitely not a `pandas.Timestamp`. + + Parameters + ---------- + **modules + Keyword-only binding from `{alias: module_name}`. + + Examples + -------- + :: + + ResultType = dict[Literal["pd", "pl"], ModuleType | None] + r: ResultType = _get_optional_modules(pd="pandas", pl="polars") + """ + return {k: sys.modules.get(v) for k, v in modules.items()} + + def _replace_parsed_shorthand( parsed_shorthand: dict[str, Any], kwds: dict[str, Any] ) -> dict[str, Any]: From 1a51948e1932319937c037b6224ca1412ac96a44 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:37:33 +0100 Subject: [PATCH 11/18] feat(typing): Update `api._top_schema_base` to work with `super(..., copy)` The call to `.to_dict`, and 3 line comment also appeared in the traceback for validation errors on `ChartType`s. This removes the need for the comment and type ignore that it is describing. See #3480 for more info on the introduction of `_top_schema_base` --- altair/vegalite/v5/api.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 5a754a9ad..a55c7be4b 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -1712,9 +1712,27 @@ def _top_schema_base( # noqa: ANN202 """ Enforces an intersection type w/ `SchemaBase` & `TopLevelMixin` objects. - Use for instance methods. + Use for methods, called from `TopLevelMixin` that are defined in `SchemaBase`. + + Notes + ----- + - The `super` sub-branch is not statically checked *here*. + - It would widen the inferred intersection to: + - `( | super)` + - Both dunder attributes are not in the `super` type stubs + - Requiring 2x *# type: ignore[attr-defined]* + - However it is required at runtime for any cases that use `super(..., copy)`. + - The inferred type **is** used statically **outside** of this function. """ - if isinstance(obj, core.SchemaBase) and isinstance(obj, TopLevelMixin): + SchemaBase = core.SchemaBase + if (isinstance(obj, SchemaBase) and isinstance(obj, TopLevelMixin)) or ( + not TYPE_CHECKING + and ( + isinstance(obj, super) + and issubclass(obj.__self_class__, SchemaBase) + and obj.__thisclass__ is TopLevelMixin + ) + ): return obj else: msg = f"{type(obj).__name__!r} does not derive from {type(core.SchemaBase).__name__!r}" @@ -1803,10 +1821,7 @@ def to_dict( # remaining to_dict calls are not at top level context["top_level"] = False - # TopLevelMixin instance does not necessarily have to_dict defined - # but due to how Altair is set up this should hold. - # Too complex to type hint right now - vegalite_spec: Any = super(TopLevelMixin, copy).to_dict( # type: ignore[misc] + vegalite_spec: Any = _top_schema_base(super(TopLevelMixin, copy)).to_dict( validate=validate, ignore=ignore, context=dict(context, pre_transform=False) ) From 48162edf9a397e327df3abaead47d07f8268bd26 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:39:43 +0100 Subject: [PATCH 12/18] fix: Use class name instead of metaclass name in error Discovered during debugging that the rhs always evaluated as `type`, which was not intentional --- altair/vegalite/v5/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index a55c7be4b..8d28a0dcc 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -1735,7 +1735,7 @@ def _top_schema_base( # noqa: ANN202 ): return obj else: - msg = f"{type(obj).__name__!r} does not derive from {type(core.SchemaBase).__name__!r}" + msg = f"{type(obj).__name__!r} does not derive from {SchemaBase.__name__!r}" raise TypeError(msg) From 788dca4e46b9f1277696df17676a10c8de8e3757 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 9 Aug 2024 11:50:44 +0100 Subject: [PATCH 13/18] docs: Propagate `to_dict` changes to `from_dict`, `to_json` Just to keep things consistent --- altair/utils/schemapi.py | 30 +++++++++++------------------- tools/schemapi/schemapi.py | 30 +++++++++++------------------- 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 7448974c0..eb7cef52f 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1052,30 +1052,27 @@ def to_json( Parameters ---------- validate : bool, optional - If True (default), then validate the output dictionary - against the schema. + If True (default), then validate the result against the schema. indent : int, optional The number of spaces of indentation to use. The default is 2. sort_keys : bool, optional If True (default), sort keys in the output. ignore : list[str], optional - A list of keys to ignore. It is usually not needed - to specify this argument as a user. + A list of keys to ignore. context : dict[str, Any], optional - A context dictionary. It is usually not needed - to specify this argument as a user. + A context dictionary. **kwargs Additional keyword arguments are passed to ``json.dumps()`` + Raises + ------ + SchemaValidationError : + If ``validate`` and the result does not conform to the schema. + Notes ----- - Technical: The ignore parameter will *not* be passed to child to_dict - function calls. - - Returns - ------- - str - The JSON specification of the chart object. + - ``ignore``, ``context`` are usually not needed to be specified as a user. + - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ if ignore is None: ignore = [] @@ -1103,15 +1100,10 @@ def from_dict( validate : boolean If True (default), then validate the input against the schema. - Returns - ------- - obj : Schema object - The wrapped schema - Raises ------ jsonschema.ValidationError : - if validate=True and dct does not conform to the schema + If ``validate`` and ``dct`` does not conform to the schema """ if validate: cls.validate(dct) diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index 34e24031f..99a0218ed 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1050,30 +1050,27 @@ def to_json( Parameters ---------- validate : bool, optional - If True (default), then validate the output dictionary - against the schema. + If True (default), then validate the result against the schema. indent : int, optional The number of spaces of indentation to use. The default is 2. sort_keys : bool, optional If True (default), sort keys in the output. ignore : list[str], optional - A list of keys to ignore. It is usually not needed - to specify this argument as a user. + A list of keys to ignore. context : dict[str, Any], optional - A context dictionary. It is usually not needed - to specify this argument as a user. + A context dictionary. **kwargs Additional keyword arguments are passed to ``json.dumps()`` + Raises + ------ + SchemaValidationError : + If ``validate`` and the result does not conform to the schema. + Notes ----- - Technical: The ignore parameter will *not* be passed to child to_dict - function calls. - - Returns - ------- - str - The JSON specification of the chart object. + - ``ignore``, ``context`` are usually not needed to be specified as a user. + - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ if ignore is None: ignore = [] @@ -1101,15 +1098,10 @@ def from_dict( validate : boolean If True (default), then validate the input against the schema. - Returns - ------- - obj : Schema object - The wrapped schema - Raises ------ jsonschema.ValidationError : - if validate=True and dct does not conform to the schema + If ``validate`` and ``dct`` does not conform to the schema """ if validate: cls.validate(dct) From b86bd2fe34556f0a840e13f3026df3389c9c5da4 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 9 Aug 2024 11:53:48 +0100 Subject: [PATCH 14/18] docs: Propagate changes to `api.(TopLevelMixin|Chart)` Should achieve greater consistency across the public API --- altair/vegalite/v5/api.py | 112 +++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 61 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 8d28a0dcc..d0b5949d4 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -1749,7 +1749,7 @@ def to_dict( self, validate: bool = True, *, - format: str = "vega-lite", + format: Literal["vega-lite", "vega"] = "vega-lite", ignore: list[str] | None = None, context: dict[str, Any] | None = None, ) -> dict[str, Any]: @@ -1759,31 +1759,25 @@ def to_dict( Parameters ---------- validate : bool, optional - If True (default), then validate the output dictionary - against the schema. - format : str, optional - Chart specification format, one of "vega-lite" (default) or "vega" + If True (default), then validate the result against the schema. + format : {"vega-lite", "vega"}, optional + The chart specification format. + The `"vega"` format relies on the active Vega-Lite compiler plugin, which + by default requires the vl-convert-python package. ignore : list[str], optional - A list of keys to ignore. It is usually not needed - to specify this argument as a user. + A list of keys to ignore. context : dict[str, Any], optional - A context dictionary. It is usually not needed - to specify this argument as a user. - - Notes - ----- - Technical: The ignore parameter will *not* be passed to child to_dict - function calls. - - Returns - ------- - dict - The dictionary representation of this chart + A context dictionary. Raises ------ - SchemaValidationError - if validate=True and the dict does not conform to the schema + SchemaValidationError : + If ``validate`` and the result does not conform to the schema. + + Notes + ----- + - ``ignore``, ``context`` are usually not needed to be specified as a user. + - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ # Validate format if format not in {"vega-lite", "vega"}: @@ -1873,7 +1867,7 @@ def to_json( indent: int | str | None = 2, sort_keys: bool = True, *, - format: str = "vega-lite", + format: Literal["vega-lite", "vega"] = "vega-lite", ignore: list[str] | None = None, context: dict[str, Any] | None = None, **kwargs: Any, @@ -1884,24 +1878,31 @@ def to_json( Parameters ---------- validate : bool, optional - If True (default), then validate the output dictionary - against the schema. + If True (default), then validate the result against the schema. indent : int, optional The number of spaces of indentation to use. The default is 2. sort_keys : bool, optional If True (default), sort keys in the output. - format : str, optional - The chart specification format. One of "vega-lite" (default) or "vega". - The "vega" format relies on the active Vega-Lite compiler plugin, which + format : {"vega-lite", "vega"}, optional + The chart specification format. + The `"vega"` format relies on the active Vega-Lite compiler plugin, which by default requires the vl-convert-python package. ignore : list[str], optional - A list of keys to ignore. It is usually not needed - to specify this argument as a user. + A list of keys to ignore. context : dict[str, Any], optional - A context dictionary. It is usually not needed - to specify this argument as a user. + A context dictionary. **kwargs Additional keyword arguments are passed to ``json.dumps()`` + + Raises + ------ + SchemaValidationError : + If ``validate`` and the result does not conform to the schema. + + Notes + ----- + - ``ignore``, ``context`` are usually not needed to be specified as a user. + - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ if ignore is None: ignore = [] @@ -3708,24 +3709,19 @@ def from_dict( cls: type[_TSchemaBase], dct: dict[str, Any], validate: bool = True ) -> _TSchemaBase: """ - Construct class from a dictionary representation. + Construct a ``Chart`` from a dictionary representation. Parameters ---------- dct : dictionary - The dict from which to construct the class + The dict from which to construct the ``Chart``. validate : boolean If True (default), then validate the input against the schema. - Returns - ------- - obj : Chart object - The wrapped schema - Raises ------ jsonschema.ValidationError : - if validate=True and dct does not conform to the schema + If ``validate`` and ``dct`` does not conform to the schema """ _tp: Any for tp in TopLevelMixin.__subclasses__(): @@ -3742,41 +3738,35 @@ def to_dict( self, validate: bool = True, *, - format: str = "vega-lite", + format: Literal["vega-lite", "vega"] = "vega-lite", ignore: list[str] | None = None, context: dict[str, Any] | None = None, ) -> dict[str, Any]: """ - Convert the chart to a dictionary suitable for JSON export. + Convert the ``Chart`` to a dictionary suitable for JSON export. Parameters ---------- validate : bool, optional - If True (default), then validate the output dictionary - against the schema. - format : str, optional - Chart specification format, one of "vega-lite" (default) or "vega" + If True (default), then validate the result against the schema. + format : {"vega-lite", "vega"}, optional + The chart specification format. + The `"vega"` format relies on the active Vega-Lite compiler plugin, which + by default requires the vl-convert-python package. ignore : list[str], optional - A list of keys to ignore. It is usually not needed - to specify this argument as a user. + A list of keys to ignore. context : dict[str, Any], optional - A context dictionary. It is usually not needed - to specify this argument as a user. - - Notes - ----- - Technical: The ignore parameter will *not* be passed to child to_dict - function calls. - - Returns - ------- - dict - The dictionary representation of this chart + A context dictionary. Raises ------ - SchemaValidationError - if validate=True and the dict does not conform to the schema + SchemaValidationError : + If ``validate`` and the result does not conform to the schema. + + Notes + ----- + - ``ignore``, ``context`` are usually not needed to be specified as a user. + - *Technical*: ``ignore`` will **not** be passed to child :meth:`.to_dict()`. """ context = context or {} kwds: Map = {"validate": validate, "format": format, "ignore": ignore, "context": context} # fmt: skip From f7f6ac2503b78c0a539ce09a87102db4b69c107e Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 9 Aug 2024 12:22:40 +0100 Subject: [PATCH 15/18] revert: Remove test added for debugging traceback length --- tests/utils/test_schemapi.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/utils/test_schemapi.py b/tests/utils/test_schemapi.py index ea45242f8..b0068724b 100644 --- a/tests/utils/test_schemapi.py +++ b/tests/utils/test_schemapi.py @@ -496,11 +496,6 @@ def chart_error_example__invalid_bandposition_value(): ) -def test_to_dict_huge_traceback(): - # Error: Invalid value for type - return alt.Chart().encode(alt.X(type="unknown")).to_dict() - - def chart_error_example__invalid_type(): # Error: Invalid value for type return alt.Chart().encode(alt.X(type="unknown")) From b42364b7d1edb38ebc6f7321efea0229d2a6cb35 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 30 Aug 2024 10:37:40 +0100 Subject: [PATCH 16/18] refactor: Use a runtime `SchemaBase` import in `api.py` https://github.com/vega/altair/pull/3530#discussion_r1735325004 --- altair/vegalite/v5/api.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 35f83fbf9..9ef5659ba 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -27,7 +27,7 @@ from altair import utils from altair.expr import core as _expr_core -from altair.utils import Optional, Undefined +from altair.utils import Optional, SchemaBase, Undefined from altair.utils._vegafusion_data import ( compile_with_vegafusion as _compile_with_vegafusion, ) @@ -125,7 +125,6 @@ ProjectionType, RepeatMapping, RepeatRef, - SchemaBase, SelectionParameter, SequenceGenerator, SortField, @@ -194,7 +193,7 @@ ] ChartDataType: TypeAlias = Optional[Union[DataType, core.Data, str, core.Generator]] -_TSchemaBase = TypeVar("_TSchemaBase", bound=core.SchemaBase) +_TSchemaBase = TypeVar("_TSchemaBase", bound=SchemaBase) # ------------------------------------------------------------------------ @@ -509,7 +508,7 @@ def check_fields_and_encodings(parameter: Parameter, field_name: str) -> bool: ] """Permitted types for `&` reduced predicates.""" -_StatementType: TypeAlias = Union[core.SchemaBase, Map, str] +_StatementType: TypeAlias = Union[SchemaBase, Map, str] """Permitted types for `if_true`/`if_false`. In python terms: @@ -532,7 +531,7 @@ def check_fields_and_encodings(parameter: Parameter, field_name: str) -> bool: _LiteralValue: TypeAlias = Union[str, bool, float, int] """Primitive python value types.""" -_FieldEqualType: TypeAlias = Union[_LiteralValue, Map, Parameter, core.SchemaBase] +_FieldEqualType: TypeAlias = Union[_LiteralValue, Map, Parameter, SchemaBase] """Permitted types for equality checks on field values: - `datum.field == ...` @@ -586,7 +585,7 @@ def _condition_to_selection( **kwargs: Any, ) -> SchemaBase | dict[str, _ConditionType | Any]: selection: SchemaBase | dict[str, _ConditionType | Any] - if isinstance(if_true, core.SchemaBase): + if isinstance(if_true, SchemaBase): if_true = if_true.to_dict() elif isinstance(if_true, str): if isinstance(if_false, str): @@ -600,7 +599,7 @@ def _condition_to_selection( if_true = utils.parse_shorthand(if_true) if_true.update(kwargs) condition.update(if_true) - if isinstance(if_false, core.SchemaBase): + if isinstance(if_false, SchemaBase): # For the selection, the channel definitions all allow selections # already. So use this SchemaBase wrapper if possible. selection = if_false.copy() @@ -662,8 +661,8 @@ def _reveal_parsed_shorthand(obj: Map, /) -> dict[str, Any]: def _is_extra(*objs: Any, kwds: Map) -> Iterator[bool]: for el in objs: - if isinstance(el, (core.SchemaBase, t.Mapping)): - item = el.to_dict(validate=False) if isinstance(el, core.SchemaBase) else el + if isinstance(el, (SchemaBase, t.Mapping)): + item = el.to_dict(validate=False) if isinstance(el, SchemaBase) else el yield not (item.keys() - kwds.keys()).isdisjoint(utils.SHORTHAND_KEYS) else: continue @@ -774,7 +773,7 @@ def _parse_literal(val: Any, /) -> dict[str, Any]: def _parse_then(statement: _StatementType, kwds: dict[str, Any], /) -> dict[str, Any]: - if isinstance(statement, core.SchemaBase): + if isinstance(statement, SchemaBase): statement = statement.to_dict() elif not isinstance(statement, dict): statement = _parse_literal(statement) @@ -786,7 +785,7 @@ def _parse_otherwise( statement: _StatementType, conditions: _Conditional[Any], kwds: dict[str, Any], / ) -> SchemaBase | _Conditional[Any]: selection: SchemaBase | _Conditional[Any] - if isinstance(statement, core.SchemaBase): + if isinstance(statement, SchemaBase): selection = statement.copy() conditions.update(**kwds) # type: ignore[call-arg] selection.condition = conditions["condition"] @@ -879,7 +878,7 @@ def then(self, statement: _StatementType, /, **kwds: Any) -> Then[Any]: return Then(_Conditional(condition=[condition])) -class Then(core.SchemaBase, t.Generic[_C]): +class Then(SchemaBase, t.Generic[_C]): """ Utility class for ``when-then-otherwise`` conditions. @@ -1728,7 +1727,6 @@ def _top_schema_base( # noqa: ANN202 - However it is required at runtime for any cases that use `super(..., copy)`. - The inferred type **is** used statically **outside** of this function. """ - SchemaBase = core.SchemaBase if (isinstance(obj, SchemaBase) and isinstance(obj, TopLevelMixin)) or ( not TYPE_CHECKING and ( @@ -3866,7 +3864,7 @@ def _check_if_valid_subspec( ], ) -> None: """Raise a `TypeError` if `spec` is not a valid sub-spec.""" - if not isinstance(spec, core.SchemaBase): + if not isinstance(spec, SchemaBase): msg = f"Only chart objects can be used in {classname}." raise TypeError(msg) for attr in TOPLEVEL_ONLY_KEYS: From bf7d5e52d7fd3401006ee952ff6b981cd8b6af0e Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 30 Aug 2024 11:04:50 +0100 Subject: [PATCH 17/18] chore: Partially restore `SchemaValidationError` comment https://github.com/vega/altair/pull/3530#discussion_r1734825467 --- altair/utils/schemapi.py | 1 + tools/schemapi/schemapi.py | 1 + 2 files changed, 2 insertions(+) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index 01b31a09b..f073d2cf9 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -1043,6 +1043,7 @@ def to_dict( raise ValueError(msg) result = _todict(kwds, context=context, **opts) if validate: + # NOTE: Don't raise `from err`, see `SchemaValidationError` doc try: self.validate(result) except jsonschema.ValidationError as err: diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index d5106503d..f06be8030 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -1041,6 +1041,7 @@ def to_dict( raise ValueError(msg) result = _todict(kwds, context=context, **opts) if validate: + # NOTE: Don't raise `from err`, see `SchemaValidationError` doc try: self.validate(result) except jsonschema.ValidationError as err: From 481790c195adcd7b9141db032c30b7919c8f9d8e Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Fri, 30 Aug 2024 12:31:24 +0100 Subject: [PATCH 18/18] docs: Improve examples for `_get_optional_modules` --- altair/utils/schemapi.py | 26 +++++++++++++++++++++----- tools/schemapi/schemapi.py | 26 +++++++++++++++++++++----- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/altair/utils/schemapi.py b/altair/utils/schemapi.py index f073d2cf9..84f5be277 100644 --- a/altair/utils/schemapi.py +++ b/altair/utils/schemapi.py @@ -58,6 +58,7 @@ from typing import Never, Self else: from typing_extensions import Never, Self + _OptionalModule: TypeAlias = "ModuleType | None" ValidationErrorList: TypeAlias = List[jsonschema.exceptions.ValidationError] GroupedValidationErrors: TypeAlias = Dict[str, ValidationErrorList] @@ -1191,7 +1192,7 @@ def __dir__(self) -> list[str]: return sorted(chain(super().__dir__(), self._kwds)) -def _get_optional_modules(**modules: str) -> dict[str, ModuleType | None]: +def _get_optional_modules(**modules: str) -> dict[str, _OptionalModule]: """ Returns packages only if they have already been imported - otherwise they return `None`. @@ -1207,10 +1208,25 @@ def _get_optional_modules(**modules: str) -> dict[str, ModuleType | None]: Examples -------- - :: - - ResultType = dict[Literal["pd", "pl"], ModuleType | None] - r: ResultType = _get_optional_modules(pd="pandas", pl="polars") + >>> import pandas as pd # doctest: +SKIP + >>> import polars as pl # doctest: +SKIP + >>> from altair.utils.schemapi import _get_optional_modules # doctest: +SKIP + >>> + >>> _get_optional_modules(pd="pandas", pl="polars", ibis="ibis") # doctest: +SKIP + { + "pd": , + "pl": , + "ibis": None, + } + + If the user later imports ``ibis``, it would appear in subsequent calls. + + >>> import ibis # doctest: +SKIP + >>> + >>> _get_optional_modules(ibis="ibis") # doctest: +SKIP + { + "ibis": , + } """ return {k: sys.modules.get(v) for k, v in modules.items()} diff --git a/tools/schemapi/schemapi.py b/tools/schemapi/schemapi.py index f06be8030..5140073ad 100644 --- a/tools/schemapi/schemapi.py +++ b/tools/schemapi/schemapi.py @@ -56,6 +56,7 @@ from typing import Never, Self else: from typing_extensions import Never, Self + _OptionalModule: TypeAlias = "ModuleType | None" ValidationErrorList: TypeAlias = List[jsonschema.exceptions.ValidationError] GroupedValidationErrors: TypeAlias = Dict[str, ValidationErrorList] @@ -1189,7 +1190,7 @@ def __dir__(self) -> list[str]: return sorted(chain(super().__dir__(), self._kwds)) -def _get_optional_modules(**modules: str) -> dict[str, ModuleType | None]: +def _get_optional_modules(**modules: str) -> dict[str, _OptionalModule]: """ Returns packages only if they have already been imported - otherwise they return `None`. @@ -1205,10 +1206,25 @@ def _get_optional_modules(**modules: str) -> dict[str, ModuleType | None]: Examples -------- - :: - - ResultType = dict[Literal["pd", "pl"], ModuleType | None] - r: ResultType = _get_optional_modules(pd="pandas", pl="polars") + >>> import pandas as pd # doctest: +SKIP + >>> import polars as pl # doctest: +SKIP + >>> from altair.utils.schemapi import _get_optional_modules # doctest: +SKIP + >>> + >>> _get_optional_modules(pd="pandas", pl="polars", ibis="ibis") # doctest: +SKIP + { + "pd": , + "pl": , + "ibis": None, + } + + If the user later imports ``ibis``, it would appear in subsequent calls. + + >>> import ibis # doctest: +SKIP + >>> + >>> _get_optional_modules(ibis="ibis") # doctest: +SKIP + { + "ibis": , + } """ return {k: sys.modules.get(v) for k, v in modules.items()}