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.

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

Incorrect conversion to VL of empty when using parameter compositions #3598

Open
joelostblom opened this issue Sep 18, 2024 · 1 comment
Open
Labels

Comments

@joelostblom
Copy link
Contributor

What happened?

The following code works as expected:

click = alt.selection_point(empty=False)
ctrl_click = alt.selection_point(on='click[event.ctrlKey]', empty=False)

points = alt.Chart(data.cars.url).mark_point().encode(
    x='Horsepower:Q',
    y='Miles_per_Gallon:Q',
    fill=alt.condition(ctrl_click, alt.value('red'), alt.value('white')),
    size=alt.condition(click, alt.value(1000), alt.value(50))
).add_params(
    click, ctrl_click
)

points

image

Open the Chart in the Vega Editor

However, when changing the fill condition to include a parameter composition:

fill=alt.condition(ctrl_click & click, alt.value('red'), alt.value('white')),

The value empty=False is no longer respected for that encoding channel:

image

Open the Chart in the Vega Editor

In the VL spec you can see that this happens because Altair converts the parameter composition into the following VL:

"condition": {
  "test": {"and": [{"param": "param_208"}, {"param": "param_207"}]},
  "value": "red"
},

What would you like to happen instead?

The correct conversion to VL would look like this:

"condition": {
  "test": {"and": [{"param": "param_208", "empty": false}, {"param": "param_207", "empty": false}]},
  "value": "red"
},

Not sure how long this has been present, but as I show above it is not related to the resent introduction of when since it also happens with condition.

Which version of Altair are you using?

5.4.1

@dangotbanned
Copy link
Member

This is unrelated to both condition and when.

All of these Parameter methods use Parameter.name only

  • omitting Parameter.(empty|param|param_type):

def to_dict(self) -> dict[str, str | dict[str, Any]]:
if self.param_type == "variable":
return {"expr": self.name}
elif self.param_type == "selection":
nm: Any = self.name
return {"param": nm.to_dict() if hasattr(nm, "to_dict") else nm}
else:
msg = f"Unrecognized parameter type: {self.param_type}"
raise ValueError(msg)
def __invert__(self) -> SelectionPredicateComposition | Any:
if self.param_type == "selection":
return SelectionPredicateComposition({"not": {"param": self.name}})
else:
return _expr_core.OperatorMixin.__invert__(self)
def __and__(self, other: Any) -> SelectionPredicateComposition | Any:
if self.param_type == "selection":
if isinstance(other, Parameter):
other = {"param": other.name}
return SelectionPredicateComposition({"and": [{"param": self.name}, other]})
else:
return _expr_core.OperatorMixin.__and__(self, other)
def __or__(self, other: Any) -> SelectionPredicateComposition | Any:
if self.param_type == "selection":
if isinstance(other, Parameter):
other = {"param": other.name}
return SelectionPredicateComposition({"or": [{"param": self.name}, other]})
else:
return _expr_core.OperatorMixin.__or__(self, other)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants