Skip to content

feat: Allow signals/params to be passed to alt.datum[...]#3990

Open
joelostblom wants to merge 4 commits intomainfrom
feat/brackets-indexing-expr
Open

feat: Allow signals/params to be passed to alt.datum[...]#3990
joelostblom wants to merge 4 commits intomainfrom
feat/brackets-indexing-expr

Conversation

@joelostblom
Copy link
Copy Markdown
Contributor

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.

Comment thread altair/expr/core.py

def __repr__(self) -> str:
return f"{self.group}[{self.name!r}]"
return f"{self.group}[{_js_repr(self.name)}]"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

!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:
  1. 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.
  2. _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.
  3. _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.
  4. _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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support alt.datum[signal_name] in addition to "datum[signal_name]"

1 participant