From 1e85a598f0afa958c8a839e1772ea6e6b189bc74 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Thu, 4 Apr 2024 08:21:36 -0700 Subject: [PATCH 01/30] Add tooltip and legend to .interactive() --- altair/vegalite/v5/api.py | 58 +++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index f050dd1d1..09fb646dd 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3814,10 +3814,15 @@ def add_selection(self, *params) -> Self: # noqa: ANN002 return self.add_params(*params) def interactive( - self, name: str | None = None, bind_x: bool = True, bind_y: bool = True + self, + name: Optional[str] = None, + bind_x: bool = True, + bind_y: bool = True, + tooltip: bool = True, + legend: Union[bool, list] = False, + legend_opacity: tuple = (0.7, 0.1), ) -> Self: - """ - Make chart axes scales interactive. + """Add common interactive elements to the chart Parameters ---------- @@ -3825,9 +3830,15 @@ def interactive( The parameter name to use for the axes scales. This name should be unique among all parameters within the chart. bind_x : boolean, default True - If true, then bind the interactive scales to the x-axis + Bind the interactive scales to the x-axis bind_y : boolean, default True - If true, then bind the interactive scales to the y-axis + Bind the interactive scales to the y-axis + tooltip : boolean, default True, + Add a tooltip containing the encodings used in the chart + legend : boolean or list, default True + Make the legend clickable and control the opacity of the marks + legend_opacity : tuple, default (0.7, 0.1) + The default opacity values for the clicked and unclicked marks Returns ------- @@ -3840,7 +3851,42 @@ def interactive( encodings.append("x") if bind_y: encodings.append("y") - return self.add_params(selection_interval(bind="scales", encodings=encodings)) + if not legend: + return self.add_params( + selection_interval(bind="scales", encodings=encodings) + ).configure_mark(tooltip=tooltip) + else: + if not isinstance(legend, list): + # Detect common legend encodings used in the spec + legend = [ + enc + for enc in self.encoding.to_dict().keys() + if enc + in [ + "angle", + "radius", + "color", + "fill", + "shape", + "size", + "stroke", + ] + ] + legend_selection = selection_point(bind="legend", encodings=legend) + return ( + self + .configure_mark(tooltip=tooltip) + .add_params( + selection_interval(bind="scales", encodings=encodings), + legend_selection, + ).encode( + opacity=condition( + legend_selection, + value(legend_opacity[0]), + value(legend_opacity[1]), + ) + ) + ) def _check_if_valid_subspec( From e76a3e21f30fe0d0f714e01d0fe4403db55fac26 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Thu, 4 Apr 2024 08:54:16 -0700 Subject: [PATCH 02/30] Avoid using `configure` to be compatible with layered specs --- altair/vegalite/v5/api.py | 43 +++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 09fb646dd..0ec4c8976 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3819,7 +3819,7 @@ def interactive( bind_x: bool = True, bind_y: bool = True, tooltip: bool = True, - legend: Union[bool, list] = False, + legend: Union[bool, list] = True, legend_opacity: tuple = (0.7, 0.1), ) -> Self: """Add common interactive elements to the chart @@ -3836,7 +3836,9 @@ def interactive( tooltip : boolean, default True, Add a tooltip containing the encodings used in the chart legend : boolean or list, default True - Make the legend clickable and control the opacity of the marks + Make the legend clickable and control the opacity of the marks. + Can be set to a list indicating which encodings the legend + interactivity should include. legend_opacity : tuple, default (0.7, 0.1) The default opacity values for the clicked and unclicked marks @@ -3851,11 +3853,20 @@ def interactive( encodings.append("x") if bind_y: encodings.append("y") - if not legend: - return self.add_params( - selection_interval(bind="scales", encodings=encodings) - ).configure_mark(tooltip=tooltip) - else: + interactive_chart = self.add_params( + selection_interval(bind="scales", encodings=encodings) + ).copy() + # We can't simply use configure_mark since configure methods + # are not allowed in layered specs + if tooltip: + if isinstance(interactive_chart.mark, str): + interactive_chart.mark = { + "type": interactive_chart.mark, + "tooltip": tooltip, + } + else: + interactive_chart.mark.tooltip = tooltip + if legend: if not isinstance(legend, list): # Detect common legend encodings used in the spec legend = [ @@ -3873,20 +3884,16 @@ def interactive( ] ] legend_selection = selection_point(bind="legend", encodings=legend) - return ( - self - .configure_mark(tooltip=tooltip) - .add_params( - selection_interval(bind="scales", encodings=encodings), + interactive_chart = interactive_chart.add_params( + legend_selection, + ).encode( + opacity=condition( legend_selection, - ).encode( - opacity=condition( - legend_selection, - value(legend_opacity[0]), - value(legend_opacity[1]), - ) + value(legend_opacity[0]), + value(legend_opacity[1]), ) ) + return interactive_chart def _check_if_valid_subspec( From 7c127fdca732fe0c2b1291f5f9768f1cf6dacae4 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Thu, 4 Apr 2024 08:55:02 -0700 Subject: [PATCH 03/30] Avoid calling `to_dict()` since `validate=False` is currently not supported for channels --- altair/vegalite/v5/api.py | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 0ec4c8976..6afb91012 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3868,21 +3868,31 @@ def interactive( interactive_chart.mark.tooltip = tooltip if legend: if not isinstance(legend, list): - # Detect common legend encodings used in the spec + # Set the legend to commonly used encodings by default legend = [ - enc - for enc in self.encoding.to_dict().keys() - if enc - in [ - "angle", - "radius", - "color", - "fill", - "shape", - "size", - "stroke", - ] + "angle", + "radius", + "color", + "fill", + "shape", + "size", + "stroke", ] + # Detect common legend encodings used in the spec + # legend = [ + # enc + # for enc in interactive_chart.encoding.to_dict(validate=False).keys() + # if enc + # in [ + # "angle", + # "radius", + # "color", + # "fill", + # "shape", + # "size", + # "stroke", + # ] + # ] legend_selection = selection_point(bind="legend", encodings=legend) interactive_chart = interactive_chart.add_params( legend_selection, From d20ecc8a09f5051bf23de7a0e64e5a34d1101586 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 22:21:45 +0200 Subject: [PATCH 04/30] Detect encodings used in spec from possible legend encodings There are many encodings that are never used in the legend which we do not need to include --- altair/vegalite/v5/api.py | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 6afb91012..57b580c41 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3820,7 +3820,6 @@ def interactive( bind_y: bool = True, tooltip: bool = True, legend: Union[bool, list] = True, - legend_opacity: tuple = (0.7, 0.1), ) -> Self: """Add common interactive elements to the chart @@ -3839,8 +3838,6 @@ def interactive( Make the legend clickable and control the opacity of the marks. Can be set to a list indicating which encodings the legend interactivity should include. - legend_opacity : tuple, default (0.7, 0.1) - The default opacity values for the clicked and unclicked marks Returns ------- @@ -3878,30 +3875,12 @@ def interactive( "size", "stroke", ] - # Detect common legend encodings used in the spec - # legend = [ - # enc - # for enc in interactive_chart.encoding.to_dict(validate=False).keys() - # if enc - # in [ - # "angle", - # "radius", - # "color", - # "fill", - # "shape", - # "size", - # "stroke", - # ] - # ] - legend_selection = selection_point(bind="legend", encodings=legend) - interactive_chart = interactive_chart.add_params( - legend_selection, - ).encode( - opacity=condition( - legend_selection, - value(legend_opacity[0]), - value(legend_opacity[1]), - ) + defined_legend_encodings = [ + enc for enc in possible_legend_encodings + if not isinstance(interactive_chart.encoding[enc], utils.schemapi.UndefinedType) + ] + else: + defined_legend_encodings = legend ) return interactive_chart From 7fea185d85866c3424afb1ad33bdfb1f932d7e65 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 22:21:59 +0200 Subject: [PATCH 05/30] Indicate which legend encodings are currently working --- altair/vegalite/v5/api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 57b580c41..c3c6102a1 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3866,14 +3866,14 @@ def interactive( if legend: if not isinstance(legend, list): # Set the legend to commonly used encodings by default - legend = [ - "angle", - "radius", + possible_legend_encodings = [ "color", "fill", "shape", - "size", "stroke", + "angle", # TODO Untested + "radius", # TODO Untested + # "size", # TODO Currently size is not working, renders empty legend ] defined_legend_encodings = [ enc for enc in possible_legend_encodings From 343bb946c1d9f3ae9f499d9ec35218901f339c23 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 22:23:23 +0200 Subject: [PATCH 06/30] Switch to using the `react: false` approach This allows us to precompute the domain without access to the data in the python code. One current issue is that that domain is not sorted, which we will need to deal with in a follow up. --- altair/vegalite/v5/api.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index c3c6102a1..5654e6eaa 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3881,7 +3881,40 @@ def interactive( ] else: defined_legend_encodings = legend + legend_selection = selection_point( + bind="legend", + encodings=defined_legend_encodings ) + for legend_encoding in defined_legend_encodings: + if not isinstance( + interactive_chart.encoding[legend_encoding]['type'], + utils.schemapi.UndefinedType + ): + legend_encoding_type = interactive_chart.encoding[legend_encoding]['type'] + else: + legend_encoding_type = interactive_chart.encoding[legend_encoding].to_dict( + context={'data': interactive_chart.data} + )['type'] + if legend_encoding_type == 'nominal': # TODO Ideally this would work for ordinal data too + initial_computed_domain = param(expr=f"domain('{legend_encoding}')") + nonreactive_domain = param(react=False, expr=initial_computed_domain.name) + if isinstance( + interactive_chart.encoding[legend_encoding]['scale'], + utils.schemapi.UndefinedType + ): + interactive_chart.encoding[legend_encoding]['scale'] = { + 'domain': nonreactive_domain + } + else: + interactive_chart.encoding[legend_encoding]['scale']['domain'] = nonreactive_domain + + interactive_chart = interactive_chart.add_params( + legend_selection, + initial_computed_domain, + nonreactive_domain + ).transform_filter( + legend_selection + ) return interactive_chart From 0b5b6885dc54e22d0dfdbc716967ec3ab46313da Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 22:23:41 +0200 Subject: [PATCH 07/30] Format docstring as per ruff --- altair/vegalite/v5/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 5654e6eaa..f836e703a 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3821,7 +3821,8 @@ def interactive( tooltip: bool = True, legend: Union[bool, list] = True, ) -> Self: - """Add common interactive elements to the chart + """ + Add common interactive elements to the chart. Parameters ---------- From cbf51c4bb0e1346786aefdde2ea2abba25c71a97 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 23:28:45 +0200 Subject: [PATCH 08/30] Support only one legend encoding channel Vega-Lite does not allow for more than one channel when binding a parameter to the legend --- altair/vegalite/v5/api.py | 52 ++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index f836e703a..a5916b01e 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3819,7 +3819,7 @@ def interactive( bind_x: bool = True, bind_y: bool = True, tooltip: bool = True, - legend: Union[bool, list] = True, + legend: bool | str = True, ) -> Self: """ Add common interactive elements to the chart. @@ -3828,22 +3828,21 @@ def interactive( ---------- name : string The parameter name to use for the axes scales. This name should be - unique among all parameters within the chart. + unique among all parameters within the chart bind_x : boolean, default True Bind the interactive scales to the x-axis bind_y : boolean, default True Bind the interactive scales to the y-axis tooltip : boolean, default True, Add a tooltip containing the encodings used in the chart - legend : boolean or list, default True - Make the legend clickable and control the opacity of the marks. - Can be set to a list indicating which encodings the legend - interactivity should include. + legend : boolean or string, default True + A single encoding channel to be used to create a clickable legend. + The deafult is to guess from the spec based on the most commonly used legend encodings. Returns ------- chart : - copy of self, with interactive axes added + copy of self, with interactivity added """ encodings: list[SingleDefUnitChannel_T] = [] @@ -3864,8 +3863,11 @@ def interactive( } else: interactive_chart.mark.tooltip = tooltip + if legend: - if not isinstance(legend, list): + if isinstance(legend, str): + legend_encoding = legend + else: # Set the legend to commonly used encodings by default possible_legend_encodings = [ "color", @@ -3876,36 +3878,36 @@ def interactive( "radius", # TODO Untested # "size", # TODO Currently size is not working, renders empty legend ] - defined_legend_encodings = [ - enc for enc in possible_legend_encodings - if not isinstance(interactive_chart.encoding[enc], utils.schemapi.UndefinedType) - ] - else: - defined_legend_encodings = legend - legend_selection = selection_point( - bind="legend", - encodings=defined_legend_encodings - ) - for legend_encoding in defined_legend_encodings: - if not isinstance( + legend_encoding = next( + ( + enc for enc in possible_legend_encodings + if not isinstance(interactive_chart.encoding[enc], utils.schemapi.UndefinedType) + ), + None + ) + + if legend_encoding is not None: + if isinstance( interactive_chart.encoding[legend_encoding]['type'], utils.schemapi.UndefinedType ): - legend_encoding_type = interactive_chart.encoding[legend_encoding]['type'] - else: legend_encoding_type = interactive_chart.encoding[legend_encoding].to_dict( context={'data': interactive_chart.data} )['type'] + else: + legend_encoding_type = interactive_chart.encoding[legend_encoding]['type'] if legend_encoding_type == 'nominal': # TODO Ideally this would work for ordinal data too + legend_selection = selection_point( + bind="legend", + encodings=[legend_encoding] + ) initial_computed_domain = param(expr=f"domain('{legend_encoding}')") nonreactive_domain = param(react=False, expr=initial_computed_domain.name) if isinstance( interactive_chart.encoding[legend_encoding]['scale'], utils.schemapi.UndefinedType ): - interactive_chart.encoding[legend_encoding]['scale'] = { - 'domain': nonreactive_domain - } + interactive_chart.encoding[legend_encoding]['scale'] = {'domain': nonreactive_domain} else: interactive_chart.encoding[legend_encoding]['scale']['domain'] = nonreactive_domain From 29994bebc25d73bc7bb81d26bc81e573bf809b41 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 23:36:02 +0200 Subject: [PATCH 09/30] Ruffify --- altair/vegalite/v5/api.py | 55 +++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index a5916b01e..36328fde7 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3880,44 +3880,53 @@ def interactive( ] legend_encoding = next( ( - enc for enc in possible_legend_encodings - if not isinstance(interactive_chart.encoding[enc], utils.schemapi.UndefinedType) + enc + for enc in possible_legend_encodings + if not isinstance( + interactive_chart.encoding[enc], + utils.schemapi.UndefinedType, + ) ), - None + None, ) if legend_encoding is not None: if isinstance( - interactive_chart.encoding[legend_encoding]['type'], - utils.schemapi.UndefinedType + interactive_chart.encoding[legend_encoding]["type"], + utils.schemapi.UndefinedType, ): - legend_encoding_type = interactive_chart.encoding[legend_encoding].to_dict( - context={'data': interactive_chart.data} - )['type'] + legend_encoding_type = interactive_chart.encoding[ + legend_encoding + ].to_dict(context={"data": interactive_chart.data})["type"] else: - legend_encoding_type = interactive_chart.encoding[legend_encoding]['type'] - if legend_encoding_type == 'nominal': # TODO Ideally this would work for ordinal data too + legend_encoding_type = interactive_chart.encoding[legend_encoding][ + "type" + ] + if ( + legend_encoding_type == "nominal" + ): # TODO Ideally this would work for ordinal data too legend_selection = selection_point( - bind="legend", - encodings=[legend_encoding] + bind="legend", encodings=[legend_encoding] ) initial_computed_domain = param(expr=f"domain('{legend_encoding}')") - nonreactive_domain = param(react=False, expr=initial_computed_domain.name) + nonreactive_domain = param( + react=False, expr=initial_computed_domain.name + ) if isinstance( - interactive_chart.encoding[legend_encoding]['scale'], - utils.schemapi.UndefinedType + interactive_chart.encoding[legend_encoding]["scale"], + utils.schemapi.UndefinedType, ): - interactive_chart.encoding[legend_encoding]['scale'] = {'domain': nonreactive_domain} + interactive_chart.encoding[legend_encoding]["scale"] = { + "domain": nonreactive_domain + } else: - interactive_chart.encoding[legend_encoding]['scale']['domain'] = nonreactive_domain + interactive_chart.encoding[legend_encoding]["scale"][ + "domain" + ] = nonreactive_domain interactive_chart = interactive_chart.add_params( - legend_selection, - initial_computed_domain, - nonreactive_domain - ).transform_filter( - legend_selection - ) + legend_selection, initial_computed_domain, nonreactive_domain + ).transform_filter(legend_selection) return interactive_chart From 8d85e02d30cb670db060a7366fac9126d8c41917 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 23:41:47 +0200 Subject: [PATCH 10/30] Note that name can be None --- 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 36328fde7..4ca95aeed 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3815,7 +3815,7 @@ def add_selection(self, *params) -> Self: # noqa: ANN002 def interactive( self, - name: Optional[str] = None, + name: Optional[str] | None = None, bind_x: bool = True, bind_y: bool = True, tooltip: bool = True, From f3a5a1770cff624844505bce8614208a882e4cb1 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Fri, 9 Aug 2024 23:50:05 +0200 Subject: [PATCH 11/30] Ignore mypy type error for `next` built-in --- 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 4ca95aeed..a002f9855 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3887,7 +3887,7 @@ def interactive( utils.schemapi.UndefinedType, ) ), - None, + None, # type: ignore ) if legend_encoding is not None: From f2305bd3561f78264df04c662cce70818cb814ea Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sat, 10 Aug 2024 00:06:49 +0200 Subject: [PATCH 12/30] Add additional legend encoding channel and indicate types --- altair/vegalite/v5/api.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index a002f9855..bcfc83021 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3819,14 +3819,29 @@ def interactive( bind_x: bool = True, bind_y: bool = True, tooltip: bool = True, - legend: bool | str = True, + legend: bool + | Literal[ + "radius", + "radius2", + "color", + "fill", + "stroke", + "opacity", + "fillOpacity", + "strokeOpacity", + "strokeWidth", + "strokeDash", + "size", + "angle", + "shape", + ] = True, ) -> Self: """ Add common interactive elements to the chart. Parameters ---------- - name : string + name : string or None The parameter name to use for the axes scales. This name should be unique among all parameters within the chart bind_x : boolean, default True @@ -3874,6 +3889,11 @@ def interactive( "fill", "shape", "stroke", + "opacity", + "fillOpacity", + "strokeOpacity", + "strokeWidth", + "strokeDash", "angle", # TODO Untested "radius", # TODO Untested # "size", # TODO Currently size is not working, renders empty legend From 2981860071b69e3255c49a145cc0d12a2e6a0232 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sun, 11 Aug 2024 10:13:14 +0200 Subject: [PATCH 13/30] Use `is_undefined` instead of `isinstance` Co-authored-by: Dan Redding <125183946+dangotbanned@users.noreply.github.com> --- altair/vegalite/v5/api.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index bcfc83021..0e9584264 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3902,18 +3902,14 @@ def interactive( ( enc for enc in possible_legend_encodings - if not isinstance( - interactive_chart.encoding[enc], - utils.schemapi.UndefinedType, - ) + if not utils.is_undefined(interactive_chart.encoding[enc]) ), None, # type: ignore ) if legend_encoding is not None: - if isinstance( - interactive_chart.encoding[legend_encoding]["type"], - utils.schemapi.UndefinedType, + if utils.is_undefined( + interactive_chart.encoding[legend_encoding]["type"] ): legend_encoding_type = interactive_chart.encoding[ legend_encoding @@ -3932,9 +3928,8 @@ def interactive( nonreactive_domain = param( react=False, expr=initial_computed_domain.name ) - if isinstance( - interactive_chart.encoding[legend_encoding]["scale"], - utils.schemapi.UndefinedType, + if utils.is_undefined( + interactive_chart.encoding[legend_encoding]["scale"] ): interactive_chart.encoding[legend_encoding]["scale"] = { "domain": nonreactive_domain From 32f900e5c85912f36dae5a7304d64a4dee3691c4 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sun, 11 Aug 2024 10:13:35 +0200 Subject: [PATCH 14/30] Simplify types Co-authored-by: Dan Redding <125183946+dangotbanned@users.noreply.github.com> --- 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 0e9584264..979bc48d4 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3815,7 +3815,7 @@ def add_selection(self, *params) -> Self: # noqa: ANN002 def interactive( self, - name: Optional[str] | None = None, + name: str | None = None, bind_x: bool = True, bind_y: bool = True, tooltip: bool = True, From cc3df47c11d4469e454074e093c6a3001b455eb9 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sun, 11 Aug 2024 10:22:40 +0200 Subject: [PATCH 15/30] Skip checking function complexity --- 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 979bc48d4..06522fb4f 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3813,7 +3813,7 @@ def add_selection(self, *params) -> Self: # noqa: ANN002 """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*params) - def interactive( + def interactive( # noqa: C901 self, name: str | None = None, bind_x: bool = True, From 706f37484c7c7372791d821b253026d65b5bc8a9 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sun, 11 Aug 2024 10:37:47 +0200 Subject: [PATCH 16/30] Don't expect str type Co-authored-by: Dan Redding <125183946+dangotbanned@users.noreply.github.com> --- 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 06522fb4f..ff246523a 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3884,7 +3884,7 @@ def interactive( # noqa: C901 legend_encoding = legend else: # Set the legend to commonly used encodings by default - possible_legend_encodings = [ + possible_legend_encodings: SingleDefUnitChannel_T = [ "color", "fill", "shape", From 3eb743e61e3bf30137b39adfce3f1ff33d3199dc Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sun, 11 Aug 2024 10:53:49 +0200 Subject: [PATCH 17/30] Adjust type to indicate that it is a list of channels --- 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 ff246523a..4cd1b9744 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3884,7 +3884,7 @@ def interactive( # noqa: C901 legend_encoding = legend else: # Set the legend to commonly used encodings by default - possible_legend_encodings: SingleDefUnitChannel_T = [ + possible_legend_encodings: list[SingleDefUnitChannel_T] = [ "color", "fill", "shape", From 4794028c01ef9eb4e6690facad817740aedbbc78 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sun, 11 Aug 2024 11:13:14 +0200 Subject: [PATCH 18/30] Indicate that there is only a subset of encodings allowed for the legend rather than all availabe ones --- altair/vegalite/v5/api.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 4cd1b9744..7c7987b7c 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3821,19 +3821,19 @@ def interactive( # noqa: C901 tooltip: bool = True, legend: bool | Literal[ - "radius", - "radius2", "color", "fill", + "shape", "stroke", "opacity", "fillOpacity", "strokeOpacity", "strokeWidth", "strokeDash", - "size", - "angle", - "shape", + "angle", # TODO Untested + "radius", # TODO Untested + "radius2", # TODO Untested + # "size", # TODO Currently size is not working, renders empty legend ] = True, ) -> Self: """ @@ -3884,7 +3884,23 @@ def interactive( # noqa: C901 legend_encoding = legend else: # Set the legend to commonly used encodings by default - possible_legend_encodings: list[SingleDefUnitChannel_T] = [ + possible_legend_encodings: list[ + Literal[ + "color", + "fill", + "shape", + "stroke", + "opacity", + "fillOpacity", + "strokeOpacity", + "strokeWidth", + "strokeDash", + "angle", # TODO Untested + "radius", # TODO Untested + "radius2", # TODO Untested + # "size", # TODO Currently size is not working, renders empty legend + ] + ] = [ "color", "fill", "shape", @@ -3896,6 +3912,7 @@ def interactive( # noqa: C901 "strokeDash", "angle", # TODO Untested "radius", # TODO Untested + "radius2", # TODO Untested # "size", # TODO Currently size is not working, renders empty legend ] legend_encoding = next( From a8001da0245ad5d7515c2a90877c7b7cb2b91ce0 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sun, 11 Aug 2024 14:44:08 +0100 Subject: [PATCH 19/30] refactor: Simplify typing, split up `.interactive()` for reuse --- altair/vegalite/v5/api.py | 187 +++++++++++++++++--------------------- 1 file changed, 81 insertions(+), 106 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 7c7987b7c..9bf76925d 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3813,29 +3813,14 @@ def add_selection(self, *params) -> Self: # noqa: ANN002 """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*params) - def interactive( # noqa: C901 + def interactive( self, name: str | None = None, bind_x: bool = True, bind_y: bool = True, tooltip: bool = True, - legend: bool - | Literal[ - "color", - "fill", - "shape", - "stroke", - "opacity", - "fillOpacity", - "strokeOpacity", - "strokeWidth", - "strokeDash", - "angle", # TODO Untested - "radius", # TODO Untested - "radius2", # TODO Untested - # "size", # TODO Currently size is not working, renders empty legend - ] = True, - ) -> Self: + legend: bool | LegendChannel_T = False, + ) -> Chart: """ Add common interactive elements to the chart. @@ -3865,101 +3850,81 @@ def interactive( # noqa: C901 encodings.append("x") if bind_y: encodings.append("y") - interactive_chart = self.add_params( + chart: Chart = self.copy().add_params( selection_interval(bind="scales", encodings=encodings) - ).copy() + ) # We can't simply use configure_mark since configure methods # are not allowed in layered specs if tooltip: - if isinstance(interactive_chart.mark, str): - interactive_chart.mark = { - "type": interactive_chart.mark, - "tooltip": tooltip, - } - else: - interactive_chart.mark.tooltip = tooltip - + chart = _add_tooltip(chart) if legend: - if isinstance(legend, str): - legend_encoding = legend - else: - # Set the legend to commonly used encodings by default - possible_legend_encodings: list[ - Literal[ - "color", - "fill", - "shape", - "stroke", - "opacity", - "fillOpacity", - "strokeOpacity", - "strokeWidth", - "strokeDash", - "angle", # TODO Untested - "radius", # TODO Untested - "radius2", # TODO Untested - # "size", # TODO Currently size is not working, renders empty legend - ] - ] = [ - "color", - "fill", - "shape", - "stroke", - "opacity", - "fillOpacity", - "strokeOpacity", - "strokeWidth", - "strokeDash", - "angle", # TODO Untested - "radius", # TODO Untested - "radius2", # TODO Untested - # "size", # TODO Currently size is not working, renders empty legend - ] - legend_encoding = next( - ( - enc - for enc in possible_legend_encodings - if not utils.is_undefined(interactive_chart.encoding[enc]) - ), - None, # type: ignore + facet_encoding: FacetedEncoding = chart.encoding + if not isinstance(legend, str): + legend = _infer_legend_encoding(facet_encoding) + + facet_legend = facet_encoding[legend] + legend_type = facet_legend["type"] + if utils.is_undefined(legend_type): + legend_type = facet_legend.to_dict(context={"data": chart.data})["type"] + + if legend_type == "nominal": + # TODO Ideally this would work for ordinal data too + legend_selection = selection_point(bind="legend", encodings=[legend]) + initial_computed_domain = param(expr=f"domain('{legend}')") + nonreactive_domain = param( + react=False, expr=initial_computed_domain.name ) - - if legend_encoding is not None: - if utils.is_undefined( - interactive_chart.encoding[legend_encoding]["type"] - ): - legend_encoding_type = interactive_chart.encoding[ - legend_encoding - ].to_dict(context={"data": interactive_chart.data})["type"] + scale = facet_legend["scale"] + if utils.is_undefined(scale): + scale = {"domain": nonreactive_domain} else: - legend_encoding_type = interactive_chart.encoding[legend_encoding][ - "type" - ] - if ( - legend_encoding_type == "nominal" - ): # TODO Ideally this would work for ordinal data too - legend_selection = selection_point( - bind="legend", encodings=[legend_encoding] - ) - initial_computed_domain = param(expr=f"domain('{legend_encoding}')") - nonreactive_domain = param( - react=False, expr=initial_computed_domain.name - ) - if utils.is_undefined( - interactive_chart.encoding[legend_encoding]["scale"] - ): - interactive_chart.encoding[legend_encoding]["scale"] = { - "domain": nonreactive_domain - } - else: - interactive_chart.encoding[legend_encoding]["scale"][ - "domain" - ] = nonreactive_domain - - interactive_chart = interactive_chart.add_params( - legend_selection, initial_computed_domain, nonreactive_domain - ).transform_filter(legend_selection) - return interactive_chart + scale["domain"] = nonreactive_domain + chart = chart.add_params( + legend_selection, + initial_computed_domain, + nonreactive_domain, + ).transform_filter(legend_selection) + else: + msg = f"Expected only 'nominal' legend type but got {legend_type!r}" + raise NotImplementedError(msg) + return chart + + +LegendChannel_T: TypeAlias = Literal[ + "color", + "fill", + "shape", + "stroke", + "opacity", + "fillOpacity", + "strokeOpacity", + "strokeWidth", + "strokeDash", + "angle", # TODO Untested + "radius", # TODO Untested + "radius2", # TODO Untested + # "size", # TODO Currently size is not working, renders empty legend +] + + +def _add_tooltip(chart: _TChart, /) -> _TChart: + mark = chart.mark + if isinstance(mark, str): + mark = {"type": mark, "tooltip": True} + else: + mark.tooltip = True + return chart + + +def _infer_legend_encoding(encoding: FacetedEncoding, /) -> LegendChannel_T: + """Set the legend to commonly used encodings by default.""" + _channels = t.get_args(LegendChannel_T) + it = (ch for ch in _channels if not utils.is_undefined(encoding[ch])) + if legend := next(it, None): + return legend + else: + msg = f"Unable to infer target channel for 'legend'.\n\n{encoding!r}" + raise NotImplementedError(msg) def _check_if_valid_subspec( @@ -5039,6 +5004,16 @@ def sphere() -> SphereGenerator: return core.SphereGenerator(sphere=True) +_TChart = TypeVar( + "_TChart", + Chart, + RepeatChart, + ConcatChart, + HConcatChart, + VConcatChart, + FacetChart, + LayerChart, +) ChartType: TypeAlias = Union[ Chart, RepeatChart, ConcatChart, HConcatChart, VConcatChart, FacetChart, LayerChart ] From 49bd736a4809bf35d53efcbbb44a3fb4ea99a99e Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sat, 17 Aug 2024 08:49:38 +0200 Subject: [PATCH 20/30] Draft test suite --- tests/vegalite/v5/test_api.py | 63 +++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index 29d68d1ea..f771dc49c 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1530,3 +1530,66 @@ def test_ibis_with_date_32(): {"a": 2, "b": "2020-01-02T00:00:00"}, {"a": 3, "b": "2020-01-03T00:00:00"}, ] + + +# TODO These chart types don't all work yet +@pytest.mark.parametrize("chart_type", ["chart"]) # , "layer", "facet"]) +def test_interactive_for_chart_types(chart_type): + chart = _make_chart_type("chart") + assert chart.interactive().to_dict() + + +def test_interactive_with_no_encoding(): + # Should not raise error when there is no encoding + assert alt.Chart().mark_point().interactive().to_dict() + + +def test_interactive_tooltip_added_for_all_encodings(): + # TODO Loop through all possible encodings + # and check that tooltip interactivity is added for all of them + assert "tooltip" in alt.Chart().mark_point().interactive().to_dict()["mark"] + assert ( + "tooltip" + not in alt.Chart().mark_point().interactive(tooltip=False).to_dict()["mark"] + ) + + +def test_interactive_legend_made_interactive_for_legend_encodings(): + # TODO Loop through all possible encodings and check that legend + # interactivity is added only for the specified legend encodings + ... + + +def test_interactive_legend_made_interactiv_for_appropriate_encodings_types( + basic_chart, +): + chart = _make_chart_type("chart") + + # TODO Reverse legend=False/True once we revert the default arg to true + assert len(chart.encode(color="color:N").interactive().to_dict()["params"]) == 1 + chart_with_nominal_legend_encoding = ( + chart.encode(color="color:N").interactive(legend=True).to_dict() + ) + assert len(chart_with_nominal_legend_encoding["params"]) == 4 + for param in chart_with_nominal_legend_encoding["params"]: + if "expr" in param: + assert param["expr"] == "domain('color')" or "react" in param + + # TODO These tests currently don't work because we are raising + # when the legend is not nominal. To be updated if we change that behavior + # TODO Could change this first test if we get interactivity working with nominal + # chart_with_ordinal_legend_encoding = ( + # chart.encode(color="color:O").interactive(legend=True).to_dict() + # ) + # assert len(chart_with_ordinal_legend_encoding["params"]) == 1 + + # chart_with_quantitative_legend_encoding = ( + # chart.encode(color="color:Q").interactive(legend=True).to_dict() + # ) + # assert len(chart_with_quantitative_legend_encoding["params"]) == 1 + + +def test_interactive_legend_encoding_correctly_picked_from_multiple_encodings(): + # TODO The legend should be chosen based on the priority order + # in the list of possible legend encodings + ... From b125c19165c9260614af157a82e1a4c6585d0f2d Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sat, 17 Aug 2024 08:50:15 +0200 Subject: [PATCH 21/30] Do not try to iterate over encodings in the spec when there are none --- altair/vegalite/v5/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 9bf76925d..e96486d24 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3857,7 +3857,8 @@ def interactive( # are not allowed in layered specs if tooltip: chart = _add_tooltip(chart) - if legend: + legend_encodings_missing = utils.is_undefined(chart.encoding) + if legend and not legend_encodings_missing: facet_encoding: FacetedEncoding = chart.encoding if not isinstance(legend, str): legend = _infer_legend_encoding(facet_encoding) From 01644a27c57755482af2dfaa257ed86429b1a967 Mon Sep 17 00:00:00 2001 From: Joel Ostblom Date: Sat, 17 Aug 2024 08:50:22 +0200 Subject: [PATCH 22/30] Ensure that tooltips are added Previously the tooltip would not be added since mark was overwitten and no longer pointing to chart.mark --- altair/vegalite/v5/api.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index e96486d24..47f5b3692 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -3909,11 +3909,10 @@ def interactive( def _add_tooltip(chart: _TChart, /) -> _TChart: - mark = chart.mark - if isinstance(mark, str): - mark = {"type": mark, "tooltip": True} + if isinstance(chart.mark, str): + chart.mark = {"type": chart.mark, "tooltip": True} else: - mark.tooltip = True + chart.mark.tooltip = True return chart From 708457c3e8a7bd9d13d2141c2abbee9409b4df1e Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 17 Aug 2024 19:57:27 +0100 Subject: [PATCH 23/30] test(typing): Add annotations --- tests/vegalite/v5/test_api.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index f771dc49c..d3a1712c6 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -12,6 +12,7 @@ import tempfile from datetime import date from importlib.metadata import version as importlib_version +from typing import TYPE_CHECKING, Any, Literal import ibis import jsonschema @@ -22,7 +23,11 @@ from packaging.version import Version import altair as alt -from altair.utils.schemapi import Optional, Undefined +from altair.utils.schemapi import Undefined + +if TYPE_CHECKING: + from altair.typing import ChartType, Optional + from altair.vegalite.v5.schema._typing import SingleDefUnitChannel_T try: import vl_convert as vlc @@ -33,8 +38,12 @@ PANDAS_VERSION = Version(importlib_version("pandas")) +_MakeType = Literal[ + "layer", "hconcat", "vconcat", "concat", "facet", "facet_encoding", "repeat" +] + -def getargs(*args, **kwargs): +def getargs(*args, **kwargs) -> tuple[tuple[Any, ...], dict[str, Any]]: return args, kwargs @@ -45,7 +54,7 @@ def getargs(*args, **kwargs): } -def _make_chart_type(chart_type): +def _make_chart_type(chart_type: _MakeType) -> ChartType: data = pd.DataFrame( { "x": [28, 55, 43, 91, 81, 53, 19, 87], @@ -80,7 +89,7 @@ def _make_chart_type(chart_type): @pytest.fixture -def basic_chart(): +def basic_chart() -> alt.Chart: data = pd.DataFrame( { "a": ["A", "B", "C", "D", "E", "F", "G", "H", "I"], @@ -1533,10 +1542,7 @@ def test_ibis_with_date_32(): # TODO These chart types don't all work yet -@pytest.mark.parametrize("chart_type", ["chart"]) # , "layer", "facet"]) -def test_interactive_for_chart_types(chart_type): - chart = _make_chart_type("chart") - assert chart.interactive().to_dict() +def test_interactive_for_chart_types(chart_type: _MakeType): def test_interactive_with_no_encoding(): From 51fd920390cef17e67efc2b6ffccf32d5b147f48 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 17 Aug 2024 19:58:30 +0100 Subject: [PATCH 24/30] test: Adds `all_chart_types`, `color_data` fixtures --- tests/vegalite/v5/test_api.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index d3a1712c6..d4364d96c 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -88,6 +88,22 @@ def _make_chart_type(chart_type: _MakeType) -> ChartType: raise ValueError(msg) +@pytest.fixture( + params=( + "layer", + "hconcat", + "vconcat", + "concat", + "facet", + "facet_encoding", + "repeat", + ) +) +def all_chart_types(request) -> ChartType: + """Use the parameter name ``all_chart_types`` to automatically parameterise.""" + return _make_chart_type(request.param) + + @pytest.fixture def basic_chart() -> alt.Chart: data = pd.DataFrame( @@ -100,6 +116,18 @@ def basic_chart() -> alt.Chart: return alt.Chart(data).mark_bar().encode(x="a", y="b") +@pytest.fixture +def color_data() -> pl.DataFrame: + """10 rows, 3 columns ``"x:Q"``, ``"y:Q"``, ``"color:(N|O)"``.""" + return pl.DataFrame( + { + "x": [0.32, 0.86, 0.10, 0.30, 0.47, 0.0, 1.0, 0.91, 0.88, 0.12], + "y": [0.11, 0.33, 0.01, 0.04, 0.77, 0.1, 0.2, 0.23, 0.05, 0.29], + "color": list("ACABBCABBA"), + } + ) + + @pytest.fixture def cars(): return pd.DataFrame( From 885767a144f94c41668eba3a574a1412674ff8fa Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 17 Aug 2024 20:00:03 +0100 Subject: [PATCH 25/30] test: Add `xfail`(s) for unimplemented in `test_interactive_for_chart_types` --- tests/vegalite/v5/test_api.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index d4364d96c..2b33ea140 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1570,7 +1570,21 @@ def test_ibis_with_date_32(): # TODO These chart types don't all work yet +@pytest.mark.parametrize( + "chart_type", + [ + "chart", + pytest.param( + "layer", marks=pytest.mark.xfail(reason="Not Implemented", raises=TypeError) + ), + pytest.param( + "facet", marks=pytest.mark.xfail(reason="Not Implemented", raises=TypeError) + ), + ], +) def test_interactive_for_chart_types(chart_type: _MakeType): + chart = _make_chart_type(chart_type) + assert chart.interactive(legend=True).to_dict() # type: ignore[call-arg] def test_interactive_with_no_encoding(): From 728d3cb24b96e97c0e1bb9fcd0373bcd2aa92254 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 17 Aug 2024 20:00:38 +0100 Subject: [PATCH 26/30] test: Update `test_interactive_with_no_encoding` to use `all_chart_types` --- tests/vegalite/v5/test_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index 2b33ea140..812ed050f 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1587,9 +1587,9 @@ def test_interactive_for_chart_types(chart_type: _MakeType): assert chart.interactive(legend=True).to_dict() # type: ignore[call-arg] -def test_interactive_with_no_encoding(): +def test_interactive_with_no_encoding(all_chart_types): # Should not raise error when there is no encoding - assert alt.Chart().mark_point().interactive().to_dict() + assert all_chart_types.interactive().to_dict() def test_interactive_tooltip_added_for_all_encodings(): From 61281d7320494b81a9b03c3c48fc6782f226862f Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 17 Aug 2024 20:01:44 +0100 Subject: [PATCH 27/30] test: Use `color_data` in `test_interactive_legend_made_interactive_for_appropriate_encodings_types` --- tests/vegalite/v5/test_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index 812ed050f..bae5317d6 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1608,10 +1608,10 @@ def test_interactive_legend_made_interactive_for_legend_encodings(): ... -def test_interactive_legend_made_interactiv_for_appropriate_encodings_types( - basic_chart, -): - chart = _make_chart_type("chart") +def test_interactive_legend_made_interactive_for_appropriate_encodings_types( + color_data, +) -> None: + chart = alt.Chart(color_data).mark_point().encode(x="x", y="y") # TODO Reverse legend=False/True once we revert the default arg to true assert len(chart.encode(color="color:N").interactive().to_dict()["params"]) == 1 From 5204759701bde17fc641a003c7590c1726f9f854 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sat, 17 Aug 2024 20:03:17 +0100 Subject: [PATCH 28/30] test: Adds implementation for `test_interactive_legend_made_interactive_for_legend_encodings` The `KeyError` is for `Radius2` not having a `type` parameter. --- tests/vegalite/v5/test_api.py | 49 ++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index bae5317d6..58967942d 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1602,10 +1602,51 @@ def test_interactive_tooltip_added_for_all_encodings(): ) -def test_interactive_legend_made_interactive_for_legend_encodings(): - # TODO Loop through all possible encodings and check that legend - # interactivity is added only for the specified legend encodings - ... +@pytest.mark.parametrize( + ("encoding", "err"), + [ + ("xOffset", NotImplementedError), + ("yOffset", NotImplementedError), + ("x2", NotImplementedError), + ("y2", NotImplementedError), + ("longitude", NotImplementedError), + ("latitude", NotImplementedError), + ("longitude2", NotImplementedError), + ("latitude2", NotImplementedError), + ("theta", NotImplementedError), + ("theta2", NotImplementedError), + ("radius", None), + ("radius2", KeyError), + ("color", None), + ("fill", None), + ("stroke", None), + ("opacity", None), + ("fillOpacity", None), + ("strokeOpacity", None), + ("strokeWidth", None), + ("strokeDash", None), + ("size", NotImplementedError), + ("angle", None), + ("shape", None), + ("key", NotImplementedError), + ("text", NotImplementedError), + ("href", NotImplementedError), + ("url", NotImplementedError), + ("description", NotImplementedError), + ], +) +def test_interactive_legend_made_interactive_for_legend_encodings( + color_data, encoding: SingleDefUnitChannel_T, err: type[Exception] | None +) -> None: + """Check that legend interactivity is added only for the allowed legend encodings.""" + chart = ( + alt.Chart(color_data).mark_point().encode(x="x", y="y", **{encoding: "color"}) # type: ignore[misc] + ) + if err is None: + assert chart.interactive(legend=True).to_dict() + else: + with pytest.raises(err): + chart.interactive(legend=True).to_dict() def test_interactive_legend_made_interactive_for_appropriate_encodings_types( From c6dac9d9fb7fe4186d1c760a21c6926d9bd41d42 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Wed, 21 Aug 2024 22:14:46 +0100 Subject: [PATCH 29/30] test: Add repro for failing test https://github.com/vega/altair/pull/3394#issuecomment-2302995453 --- tests/vegalite/v5/test_api.py | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index 58967942d..549e8adc7 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1587,6 +1587,59 @@ def test_interactive_for_chart_types(chart_type: _MakeType): assert chart.interactive(legend=True).to_dict() # type: ignore[call-arg] +def test_repro_3394_2302995453(): + r""" + Related to ``SchemaBase.copy`` + + Log output:: + + tests/vegalite/v5/test_api.py:1608: + + altair/vegalite/v5/api.py:3856: in interactive + chart: Chart = self.copy().add_params( + altair/utils/schemapi.py:924: in copy + return cast("Self", _deep_copy(self, set(ignore) if ignore else set())) + altair/utils/schemapi.py:858: in _deep_copy + kwds = {k: (copy(v) if k not in by_ref else v) for k, v in obj._kwds.items()} + altair/utils/schemapi.py:858: in _deep_copy + kwds = {k: (copy(v) if k not in by_ref else v) for k, v in obj._kwds.items()} + + obj = Then({ + condition: [{'test': ((datum.IMDB_Rating === null) || (datum.Rotten_Tomatoes_Rating === null)), 'value': 'grey'}] + }), by_ref = set() + + def _deep_copy(obj: _CopyImpl | Any, by_ref: set[str]) -> _CopyImpl | Any: + copy = partial(_deep_copy, by_ref=by_ref) + if isinstance(obj, SchemaBase): + args = (copy(arg) for arg in obj._args) + kwds = {k: (copy(v) if k not in by_ref else v) for k, v in obj._kwds.items()} + with debug_mode(False): + > return obj.__class__(*args, **kwds) + E TypeError: Then.__init__() got an unexpected keyword argument 'condition' + + altair/utils/schemapi.py:860: TypeError + + """ # noqa: D400 + import altair as alt + from vega_datasets import data + + source = data.movies.url + predicate = (alt.datum.IMDB_Rating == None) | ( # noqa: E711 + alt.datum.Rotten_Tomatoes_Rating == None # noqa: E711 + ) + + chart = ( + alt.Chart(source) + .mark_point(invalid=None) + .encode( + x="IMDB_Rating:Q", + y="Rotten_Tomatoes_Rating:Q", + color=alt.when(predicate).then(alt.value("grey")), + ) + ) + chart.interactive() + + def test_interactive_with_no_encoding(all_chart_types): # Should not raise error when there is no encoding assert all_chart_types.interactive().to_dict() From bd7ccfb9ad88feaa5fdd7c2081847eb5528aa732 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Sun, 25 Aug 2024 15:24:29 +0100 Subject: [PATCH 30/30] test: Remove test case fixed by #3553 --- tests/vegalite/v5/test_api.py | 53 ----------------------------------- 1 file changed, 53 deletions(-) diff --git a/tests/vegalite/v5/test_api.py b/tests/vegalite/v5/test_api.py index 549e8adc7..58967942d 100644 --- a/tests/vegalite/v5/test_api.py +++ b/tests/vegalite/v5/test_api.py @@ -1587,59 +1587,6 @@ def test_interactive_for_chart_types(chart_type: _MakeType): assert chart.interactive(legend=True).to_dict() # type: ignore[call-arg] -def test_repro_3394_2302995453(): - r""" - Related to ``SchemaBase.copy`` - - Log output:: - - tests/vegalite/v5/test_api.py:1608: - - altair/vegalite/v5/api.py:3856: in interactive - chart: Chart = self.copy().add_params( - altair/utils/schemapi.py:924: in copy - return cast("Self", _deep_copy(self, set(ignore) if ignore else set())) - altair/utils/schemapi.py:858: in _deep_copy - kwds = {k: (copy(v) if k not in by_ref else v) for k, v in obj._kwds.items()} - altair/utils/schemapi.py:858: in _deep_copy - kwds = {k: (copy(v) if k not in by_ref else v) for k, v in obj._kwds.items()} - - obj = Then({ - condition: [{'test': ((datum.IMDB_Rating === null) || (datum.Rotten_Tomatoes_Rating === null)), 'value': 'grey'}] - }), by_ref = set() - - def _deep_copy(obj: _CopyImpl | Any, by_ref: set[str]) -> _CopyImpl | Any: - copy = partial(_deep_copy, by_ref=by_ref) - if isinstance(obj, SchemaBase): - args = (copy(arg) for arg in obj._args) - kwds = {k: (copy(v) if k not in by_ref else v) for k, v in obj._kwds.items()} - with debug_mode(False): - > return obj.__class__(*args, **kwds) - E TypeError: Then.__init__() got an unexpected keyword argument 'condition' - - altair/utils/schemapi.py:860: TypeError - - """ # noqa: D400 - import altair as alt - from vega_datasets import data - - source = data.movies.url - predicate = (alt.datum.IMDB_Rating == None) | ( # noqa: E711 - alt.datum.Rotten_Tomatoes_Rating == None # noqa: E711 - ) - - chart = ( - alt.Chart(source) - .mark_point(invalid=None) - .encode( - x="IMDB_Rating:Q", - y="Rotten_Tomatoes_Rating:Q", - color=alt.when(predicate).then(alt.value("grey")), - ) - ) - chart.interactive() - - def test_interactive_with_no_encoding(all_chart_types): # Should not raise error when there is no encoding assert all_chart_types.interactive().to_dict()