feat: Allow signals/params to be passed to alt.datum[...]#3990
feat: Allow signals/params to be passed to alt.datum[...]#3990joelostblom wants to merge 4 commits intomainfrom
Conversation
|
|
||
| def __repr__(self) -> str: | ||
| return f"{self.group}[{self.name!r}]" | ||
| return f"{self.group}[{_js_repr(self.name)}]" |
There was a problem hiding this comment.
!r calls Python's repr() unconditionally. For a string "foo" that gives 'foo' (quoted), which is correct for a literal string key. For a Parameter object it gives the full Python repr like Parameter('xcol', VariableParameter({...})) — which is meaningless as a Vega expression. Switching to _js_repr will instead call _to_expr(), which is overridden by Parameter to return self.name (the bare string "xcol"), which is exactly what Vega expects as a signal reference. For plain strings and numbers, _js_repr falls through to repr(), so those are unchanged.
Neither me, nor my agent can think of any notable caveat here: strings are rendered as before so there is no change there. There are some new nonsensical constructs that are now possible, like passing dictkeys and datetime objects, but none of them represent an actual use case I believe and they still fail as expected on the Vega-Lite level.
Some details agent notes about these cases:
- Any OperatorMixin can now be a dict key. Beyond Parameter, things like BinaryExpression or FunctionExpression also extend OperatorMixin. So datum[datum.x + 1] would now render as datum[(datum.x + 1)], which is actually valid Vega — but it's an unusual pattern nobody has likely tested. Previously it would have rendered as datum[] which was broken anyway, so this is strictly an improvement.
- _js_repr on a plain integer returns repr(int) = e.g. "0". So datum[0] renders as datum[0] — same as before since 0!r is also "0". No change.
- _js_repr on True/False/None returns "true"/"false"/"null". So datum[True] would now render as datum[true] instead of datum[True]. This is arguably more correct JavaScript, but it's an edge case nobody would realistically write.
- _js_repr on a dt.date object would render it as a datetime(...) Vega call rather than Python's repr. Again, datum[some_date] is not a realistic use case.
None of the caveats are likely to matter in practice — the only realistic index types are strings (field name literals, unchanged) and Parameter/OperatorMixin objects (signal references, now fixed). The existing test suite catches the string case explicitly.
close #3366
After this, I think we can consider using alt.expr as the default way to represent expressions in the docs instead of the js strings.