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

The parameter empty can be used in multiple places which lead to different error messages #3596

Open
joelostblom opened this issue Sep 18, 2024 · 3 comments
Labels

Comments

@joelostblom
Copy link
Contributor

What happened?

I was playing around with this gallery example when I noticed that there seems to be multiple ways of specifying empty=True inside when-then-otherwise. The default in the example is to put it inside alt.value:

alt.when(select).then(alt.value(2, empty=False))

However, it also works to put it inside then():

alt.when(select).then(alt.value(2), empty=False)

and inside when():

alt.when(select, empty=False).then(alt.value(2))

They all work they same, but somewhat confusingly, the addition of other parameters leads to different behavior between the three. For when no error is raised but the parameter is also not applied, e.g.:

alt.when(select, empty=False, nearest=True)

For the other two, an error is raised, but it points the user in the wrong direction:

alt.when(select).then(alt.value(2), empty=False, nearest=True)
SchemaValidationError: `StrokeWidth` has no parameter named 'empty'

What would you like to happen instead?

I think the ideal case would be if we did not allow for empty in then() and instead only in when which is also clearly documented currently.

I don't think there is anything we can do about value since it has always accepted kwds. But if the error message could be made clearer, then that would be helpful (maybe related to #2913). I think the clearest possible message in the case of

alt.when(select).then(alt.value(2), empty=False, nearest=True)

would be

SchemaValidationError: `When` has no parameter named 'nearest'

cc @dangotbanned since you have the most expertise here, do you think it is possible to raise this error instead of the current behavior?

Which version of Altair are you using?

5.4.1

@dangotbanned
Copy link
Member

@joelostblom two points that stood out to me:

  1. You mention empty, but the last three code blocks also contain nearest=True.
    • Is that required for the repro?
    • Also could you add the definition of select in
      • alt.when(select).then(alt.value(2, empty=False))
  2. In fix(typing): Improve Then annotations, autocompletion, docs #3567, I've improved the docs, typing and added a new exception case.

@joelostblom
Copy link
Contributor Author

joelostblom commented Sep 19, 2024

The code is from this gallery example so select is:

select = alt.selection_point(name="select", on="click")

Without nearest there is no error message. But with nearest alone there is an error message if it is passed to value, but not if it passed to when or then:

alt.when(select).then(alt.value(2, nearest=False))
SchemaValidationError: `StrokeWidth` has no parameter named 'nearest'

This error makes sense since the kwds of value are applied to strokeWidth, but it doesn't make sense that when using alt.value(2, empty=False)) the keyword empty is passed to the param; it should also raise an error since strokeWidth does not have a kwd called empty.

Could you try this on that branch please?

Will try that out later today!

@dangotbanned
Copy link
Member

Previous comment

The code is from this gallery example so select is:

select = alt.selection_point(name="select", on="click")

Without nearest there is no error message. But with nearest alone there is an error message if it is passed to value, but not if it passed to when or then:

alt.when(select).then(alt.value(2, nearest=False))
SchemaValidationError: `StrokeWidth` has no parameter named 'nearest'

This error makes sense since the kwds of value are applied to strokeWidth, but it doesn't make sense that when using alt.value(2, empty=False)) the keyword empty is passed to the param; it should also raise an error since strokeWidth does not have a kwd called empty.

Could you try this on that branch please?

Will try that out later today!

Okay I think I see the confusion @joelostblom:

Repro

import altair as alt

select = alt.selection_point(name="select", on="click")
highlight = alt.selection_point(name="highlight", on="pointerover", empty=False)
stroke_width = (
    alt.when(select)
    .then(alt.value(2, empty=False))
    .when(highlight)
    .then(alt.value(1))
    .otherwise(alt.value(0))
)

then_nearest_1 = alt.when(select).then(alt.value(2, nearest=False)).to_dict()
# Just testing `alt.value` isn't contributing to the issue
then_nearest_2 = alt.when(select).then(alt.value(2), nearest=False).to_dict()

print(stroke_width, then_nearest_1, then_nearest_2, sep="\n")

Output

# -------------------------------------------->
{'condition': [{'param': 'select', 'value': 2, 'empty': False}, {'param': 'highlight', 'empty': False, 'value': 1}], 'value': 0}
{'condition': [{'param': 'select', 'value': 2, 'nearest': False}]}
{'condition': [{'param': 'select', 'value': 2, 'nearest': False}]}

I had already added links to this page on #3567 but empty is valid (as is value) in each of those pairs.

So each element in the list keyed to {"condition": [...]} should look like this:

Code block

class _ConditionClosed(TypedDict, closed=True, total=False):  # type: ignore[call-arg]
    # https://peps.python.org/pep-0728/
    # Parameter {"param", "value", "empty"}
    # Predicate {"test", "value"}
    empty: Optional[bool]
    param: Parameter | str
    test: _TestPredicateType
    value: Any


_Conditions: TypeAlias = t.List[_ConditionClosed]
"""
Chainable conditions produced by ``.when()`` and ``Then.when()``.

All must be a `Conditional Value`_.

.. _Conditional Value:
    https://vega.github.io/vega-lite/docs/condition.html#value
"""

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