Using field predicates in conditions

[fedarko]

Hi -- first off, thanks for developing Altair! It's already been very useful for our work.

I've been looking into implementing conditional size encodings for marks in a visualization based on the value of an attribute. I figured I might do this by using a FieldEqualPredicate inside a condition, but every time I tried to use this predicate by itself in a condition I got the following error:

NotImplementedError: condition predicate of type <class 'altair.vegalite.v2.schema.core.FieldEqualPredicate'>

As an example of this, here's the Jupyter Notebook example code from the installation page, with the only addition being a size argument that results in this error. The intended behavior of this size argument would be making each point with a petalWidth of 1 larger than the other points in the plot.

import altair as alt
from vega_datasets import data

alt.renderers.enable('notebook')

iris = data.iris()

alt.Chart(iris).mark_point().encode(
    x='petalLength',
    y='petalWidth',
    color='species',
    size=alt.condition(
        alt.FieldEqualPredicate(field='petalWidth', equal=1.0),
        alt.value(30.0),
        alt.value(5.0)
    )
)

Wrapping the FieldEqualPredicate inside a Predicate makes this work, though:

...
    size=alt.condition(
        alt.Predicate(
            alt.FieldEqualPredicate(field='petalWidth', equal=1.0)
        ),
        alt.value(30.0),
        alt.value(5.0)
    )
...

Is this current behavior (of field predicates being required to be wrapped inside a general Predicate when being used in a condition) intentional? I can't find any explicit documentation of this requirement outside of the description of the predicate parameter of altair.condition() (and without some knowledge of the various Predicate object hierarchies in Altair, it seems at first like a field predicate would be usable by itself for that parameter).

If this behavior is intentional, I think it might be a good idea to update the docs to make this more clear. (As mentioned, from just looking at the condition (1) and FieldEqualPredicate (1, 2) docs, it isn't immediately clear why the first code example in this issue fails.) In either case, I'd be happy to look into making a pull request to either change this (so that field predicates can be directly passed into conditions) or update the documentation about this behavior.

Thanks again for Altair! :)

[jakevdp]

This is a bug – we need to update the list of recognized predicates here: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v2/api.py#L334-L336

[jakevdp]

Long-term, the best fix for this would be to have schemapi auto-generate the class hierarchy implied within the vega-lite schema (so all of these predicate classes would be derived from the Predicate base class) but I think that would take a fair bit of work.

[fedarko]

Got it, thanks! I agree that deriving each predicate class from Predicate would be the best option. I'm not really familiar with schemapi, so I don't know if I'd be able to work on that right now.

To address this problem in the short-term, I've submitted a PR (#1143) which adds all of the field predicates listed here to test_predicates in condition() in api.py. Let me know if this looks ok.

I have two related questions:

1. From looking through core.py, I notice that SelectionPredicate also isn't explicitly mentioned in selection_predicates and test_predicates in condition(). Should we add it to one of the predicate tuples, also? I'm not sure if there's ever a case where SelectionPredicate is used on its own in a condition.

2. I tried to test each of these field predicates on the sample dataset above, just to verify that they all worked properly without being wrapped inside a Predicate. FieldEqualPredicate, FieldOneOfPredicate, and FieldRangePredicate all seemed to work ok after the single change in Update test_predicates to include field predicates #1143. However, the other field predicates (LT, LTE, GT, GTE) caused me to get a JavaScript error when running the test in a Jupyter Notebook -- I've attached a screenshot of one of these errors.

(Strangely enough, these errors don't seem to manifest when running Altair just in a normal Python console. Not sure if this is a problem just with the interface with Jupyter Notebook.)

[jakevdp]

I think the javascript error is probably due to you having an older version of vega-lite in your jupyter notebook extension. What version of the Python vega module do you have installed?

[jakevdp]

Regarding valid options for each... the test variable has to be a valid LogicalOperand<Predicate>, defined here: https://github.com/altair-viz/altair/blob/d71af34d9b5b5e45ac0773f68553e716b5a95ff8/altair/vegalite/v2/schema/vega-lite-schema.json#L4522-L4536

The selection variable has to be a valid SelectionOperand, defined here: https://github.com/altair-viz/altair/blob/d71af34d9b5b5e45ac0773f68553e716b5a95ff8/altair/vegalite/v2/schema/vega-lite-schema.json#L4538-L4553

[jakevdp]

These definitions probably changed in one of the vega-lite updates along the way, and we forgot to update the list within the alt.condition function.

[fedarko]

I think the javascript error is probably due to you having an older version of vega-lite in your jupyter notebook extension. What version of the Python vega module do you have installed?

Yeah, vega versioning issues seem to have been the problem for that error. I had vega3 installed using conda for some reason (in addition to vega), and uninstalling vega3 fixed the problem. To test the problem, I reinstalled vega3 and restarted jupyter notebook, which caused the problem to return. (I think that, a while back, I must have just installed vega3 without realizing that it was deprecated.)

Regarding valid options for each... the test variable has to be a valid LogicalOperand<Predicate>, defined here: [...]
The selection variable has to be a valid SelectionOperand, defined here: [...]

I'm still a little confused. Since it's not included in either specification, would this mean that SelectionPredicate shouldn't be added to either of these lists? Or is its absence a problem, similar to how the field predicates were also omitted from either list?

Thanks for all of your help with this!

[jakevdp]

SelectionPredicate is in the Predicate list, so it's valid as well: https://github.com/altair-viz/altair/blob/d71af34d9b5b5e45ac0773f68553e716b5a95ff8/altair/vegalite/v2/schema/vega-lite-schema.json#L5647-L5677

[fedarko]

Got it! Ok, that makes sense. I've pushed another commit to PR #1143 that adds SelectionPredicate to the test_predicates list. I'm not sure how to go about directly testing the act of creating a SelectionPredicate and using it in a condition, but at least from running make test this commit didn't seem to break anything.