Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly specify arguments for to_dict and to_json methods for top-level chart objects #3073

Merged
merged 6 commits into from
Jun 2, 2023
Merged

Explicitly specify arguments for to_dict and to_json methods for top-level chart objects #3073

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
85fb4fa
Explicitly specify arguments for to_dict and to_json methods ofor top…
binste May 29, 2023
b19549e
Exclude Optional and TypingDict from __all__
binste May 29, 2023
86ac02e
Change boolean to bool in description
binste May 29, 2023
8b63b08
Improve docstrings
binste May 29, 2023
231a609
Another docstring improvement
binste May 29, 2023
6005f13
Move indent and sort_keys before ignore and context in to_json
binste May 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 43 additions & 28 deletions altair/utils/schemapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,29 +734,39 @@ def __eq__(self, other):
and self._kwds == other._kwds
)

def to_dict(self, validate=True, ignore=None, context=None):
def to_dict(
self,
validate: bool = True,
ignore: Optional[List[str]] = None,
context: Optional[Dict[str, Any]] = None,
) -> dict:
"""Return a dictionary representation of the object

Parameters
----------
validate : boolean
validate : bool, optional
If True (default), then validate the output dictionary
against the schema.
ignore : list
A list of keys to ignore. This will *not* passed to child to_dict
function calls.
context : dict (optional)
A context dictionary that will be passed to all child to_dict
function calls
ignore : list[str], optional
A list of keys to ignore. It is usually not needed
to specify this argument as a user.
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
-------
dct : dictionary
dict
The dictionary representation of this object

Raises
------
jsonschema.ValidationError :
SchemaValidationError :
if validate=True and the dict does not conform to the schema
"""
if context is None:
Expand Down Expand Up @@ -816,36 +826,41 @@ def to_dict(self, validate=True, ignore=None, context=None):

def to_json(
self,
validate=True,
ignore=None,
context=None,
indent=2,
sort_keys=True,
validate: bool = True,
ignore: Optional[List[str]] = None,
context: Optional[Dict[str, Any]] = None,
indent: int = 2,
sort_keys: bool = True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're changing signatures, how would you feel about moving indent and sort_keys above ignore and context. Like this:

    def to_json(
        validate: bool = True,
        indent: int = 2,
        sort_keys: bool = True,
        ignore: Optional[List[str]] = None,
        context: Optional[Dict[str, Any]] = None,

I doubt this would be breaking in practice as ignore and context aren't intended for use by end users, and even if they are being used it would be even more unusual to specify validate ignore and context as positional args.

This is top of mind because I'd rather add the format argument in #3071 before ignore and context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Changed in the latest commit.

**kwargs,
):
) -> str:
"""Emit the JSON representation for this object as a string.

Parameters
----------
validate : boolean
validate : bool, optional
If True (default), then validate the output dictionary
against the schema.
ignore : list (optional)
A list of keys to ignore. This will *not* passed to child to_dict
function calls.
context : dict (optional)
A context dictionary that will be passed to all child to_dict
function calls
indent : integer, default 2
the number of spaces of indentation to use
sort_keys : boolean, default True
if True, 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.
context : dict[str, Any], optional
A context dictionary. It is usually not needed
to specify this argument as a user.
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.
**kwargs
Additional keyword arguments are passed to ``json.dumps()``

Notes
-----
Technical: The ignore parameter will *not* be passed to child to_dict
function calls.

Returns
-------
spec : string
str
The JSON specification of the chart object.
"""
if ignore is None:
Expand Down
92 changes: 81 additions & 11 deletions altair/vegalite/v5/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
from toolz.curried import pipe as _pipe
import itertools
import sys
from typing import cast
from typing import cast, List, Optional, Any

# Have to rename it here as else it overlaps with schema.core.Type
from typing import Type as TypingType
from typing import Dict as TypingDict

from .schema import core, channels, mixins, Undefined, SCHEMA_URL

Expand Down Expand Up @@ -815,8 +816,41 @@ class TopLevelMixin(mixins.ConfigMethodMixin):

_class_is_valid_at_instantiation = False

def to_dict(self, *args, **kwargs) -> dict:
"""Convert the chart to a dictionary suitable for JSON export"""
def to_dict(
self,
validate: bool = True,
ignore: Optional[List[str]] = None,
context: Optional[TypingDict[str, Any]] = None,
) -> dict:
"""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.
ignore : list[str], optional
A list of keys to ignore. It is usually not needed
to specify this argument as a user.
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

Raises
------
SchemaValidationError
if validate=True and the dict does not conform to the schema
"""
# We make use of three context markers:
# - 'data' points to the data that should be referenced for column type
# inference.
Expand All @@ -827,7 +861,7 @@ def to_dict(self, *args, **kwargs) -> dict:

# note: not a deep copy because we want datasets and data arguments to
# be passed by reference
context = kwargs.get("context", {}).copy()
context = context.copy() if context else {}
context.setdefault("datasets", {})
is_top_level = context.get("top_level", True)

Expand All @@ -842,12 +876,13 @@ def to_dict(self, *args, **kwargs) -> dict:

# remaining to_dict calls are not at top level
context["top_level"] = False
kwargs["context"] = context

# 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
dct = super(TopLevelMixin, copy).to_dict(*args, **kwargs) # type: ignore[misc]
dct = super(TopLevelMixin, copy).to_dict( # type: ignore[misc]
validate=validate, ignore=ignore, context=context
)

# TODO: following entries are added after validation. Should they be validated?
if is_top_level:
Expand Down Expand Up @@ -2508,16 +2543,51 @@ def from_dict(cls, dct, validate=True) -> "Chart": # type: ignore[override] #
# As a last resort, try using the Root vegalite object
return core.Root.from_dict(dct, validate)

def to_dict(self, *args, **kwargs) -> dict:
"""Convert the chart to a dictionary suitable for JSON export."""
context = kwargs.get("context", {})
def to_dict(
self,
validate: bool = True,
ignore: Optional[List[str]] = None,
context: Optional[TypingDict[str, Any]] = None,
) -> dict:
"""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.
ignore : list[str], optional
A list of keys to ignore. It is usually not needed
to specify this argument as a user.
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

Raises
------
SchemaValidationError
if validate=True and the dict does not conform to the schema
"""
context = context or {}
if self.data is Undefined and "data" not in context:
# No data specified here or in parent: inject empty data
# for easier specification of datum encodings.
copy = self.copy(deep=False)
copy.data = core.InlineData(values=[{}])
return super(Chart, copy).to_dict(*args, **kwargs)
return super().to_dict(*args, **kwargs)
return super(Chart, copy).to_dict(
validate=validate, ignore=ignore, context=context
)
return super().to_dict(validate=validate, ignore=ignore, context=context)

def add_params(self, *params) -> Self:
"""Add one or more parameters to the chart."""
Expand Down
71 changes: 43 additions & 28 deletions tools/schemapi/schemapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -732,29 +732,39 @@ def __eq__(self, other):
and self._kwds == other._kwds
)

def to_dict(self, validate=True, ignore=None, context=None):
def to_dict(
self,
validate: bool = True,
ignore: Optional[List[str]] = None,
context: Optional[Dict[str, Any]] = None,
) -> dict:
"""Return a dictionary representation of the object

Parameters
----------
validate : boolean
validate : bool, optional
If True (default), then validate the output dictionary
against the schema.
ignore : list
A list of keys to ignore. This will *not* passed to child to_dict
function calls.
context : dict (optional)
A context dictionary that will be passed to all child to_dict
function calls
ignore : list[str], optional
A list of keys to ignore. It is usually not needed
to specify this argument as a user.
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
-------
dct : dictionary
dict
The dictionary representation of this object

Raises
------
jsonschema.ValidationError :
SchemaValidationError :
if validate=True and the dict does not conform to the schema
"""
if context is None:
Expand Down Expand Up @@ -814,36 +824,41 @@ def to_dict(self, validate=True, ignore=None, context=None):

def to_json(
self,
validate=True,
ignore=None,
context=None,
indent=2,
sort_keys=True,
validate: bool = True,
ignore: Optional[List[str]] = None,
context: Optional[Dict[str, Any]] = None,
indent: int = 2,
sort_keys: bool = True,
**kwargs,
):
) -> str:
"""Emit the JSON representation for this object as a string.

Parameters
----------
validate : boolean
validate : bool, optional
If True (default), then validate the output dictionary
against the schema.
ignore : list (optional)
A list of keys to ignore. This will *not* passed to child to_dict
function calls.
context : dict (optional)
A context dictionary that will be passed to all child to_dict
function calls
indent : integer, default 2
the number of spaces of indentation to use
sort_keys : boolean, default True
if True, 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.
context : dict[str, Any], optional
A context dictionary. It is usually not needed
to specify this argument as a user.
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.
**kwargs
Additional keyword arguments are passed to ``json.dumps()``

Notes
-----
Technical: The ignore parameter will *not* be passed to child to_dict
function calls.

Returns
-------
spec : string
str
The JSON specification of the chart object.
"""
if ignore is None:
Expand Down
4 changes: 3 additions & 1 deletion tools/update_init_file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sys
from pathlib import Path
from os.path import abspath, dirname, join
from typing import TypeVar, Type, cast, List, Any
from typing import TypeVar, Type, cast, List, Any, Optional

import black

Expand Down Expand Up @@ -80,6 +80,8 @@ def _is_relevant_attribute(attr_name):
or attr is List
or attr is Any
or attr is Literal
or attr is Optional
or attr_name == "TypingDict"
):
return False
else:
Expand Down