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

feat: Support Chart.transform_filter(*predicates, **constraints) #3664

Merged
merged 28 commits into from
Nov 13, 2024

Conversation

dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Oct 30, 2024

Will close #3657

Description

This PR enables the same syntax from alt.when to be used in alt.TopLevelMixin.transform_filter.

Honestly, I'm still a bit shocked by how little needed changing in the implementation.
Besides tweaking the signature and handling the deprecated argument, it really was just these 2 lines:

cond = _parse_when(predicate, *more_predicates, empty=empty, **constraints)
return self._add_transform(core.FilterTransform(filter=cond.get("test", cond)))

To explain this, _parse_when returns our version of VL-Condition.
We can either use that as-is or unwrap the "test" key, which stores an already parsed test VL-Predicate.

Besides this small change, most of the PR consists of new docs & tests.

Doc (Examples)

Setting up a common chart:

source = data.population.url
chart = (
    alt.Chart(source)
    .mark_line()
    .encode(
        x="age:O",
        y="sum(people):Q",
        color=alt.Color("year:O").legend(symbolType="square"),
    )
)
chart

tf-filter-1

Singular predicates can be expressed via datum:

chart.transform_filter(datum.year <= 1980)

tf-filter-2

We can also use selection parameters directly:

selection = alt.selection_point(encodings=["color"], bind="legend")
chart.transform_filter(selection).add_params(selection)

tf-filter-3

Or a field predicate:

between_1950_60 = alt.FieldRangePredicate(field="year", range=[1950, 1960])
chart.transform_filter(between_1950_60) | chart.transform_filter(~between_1950_60)

tf-filter-4

Predicates can be composed together using logical operands:

chart.transform_filter(between_1950_60 | (datum.year == 1850))

tf-filter-5

Predicates passed as positional arguments will be reduced with &:

chart.transform_filter(datum.year > 1980, datum.age != 90)

tf-filter-6

Using keyword-argument constraints can simplify compositions like:

verbose_composition = chart.transform_filter((datum.year == 2000) & (datum.sex == 1))
chart.transform_filter(year=2000, sex=1)

tf-filter-7

Tasks

  • Deprecate using filter as a keyword argument
  • Replace current .transform_filter() implementation
  • Review current tests/failures
    • Was expecting this to be more complicated
    • No test failures
    • One typing issue
  • Add new tests for (*predicates, **constraints) syntax (b497039)
  • Address predicate dict typing issue
  • Update .transform_filter() docstring (7a0cc42)

- Not planning to keep the new body of the method
- Purely fishing for the first test failure
Interestingly, this doesn't break any tests,
Would indicate what I suspected in (#3657) was true
Remember discovering this during (#3427), but hadn't documented
Includes deprecation handling
- Need to widen the definition of `_PredicateType`
- To support this, we'll need to model with `TypedDict`(s)
This alias may be redundant now, need to review that later
Didn't account for a `TypeError` that can be triggered if `more_predicates` isn't composable.
E.g `filter={"field": "year", "oneOf": [1955, 2000]}`
@dangotbanned dangotbanned changed the title feat(DRAFT): Support Chart.transform_filter(*predicates, **constraints) feat: Support Chart.transform_filter(*predicates, **constraints) Oct 31, 2024
- Builds on the style introdcued for `alt.when`
- Shows a few specific kinds of predicates - due to the prior doc listing 5

#3657
Need to forgo all type safety here, since it would permit `_PredicateType` instead of the narrower `_FieldEqualType`
I added this while trying to resolve a different typing issue, purely to see what was inferred
Think this makes it clearer how each of these align
@dangotbanned dangotbanned marked this pull request as ready for review November 4, 2024 13:43
@mattijn
Copy link
Contributor

mattijn commented Nov 13, 2024

Thanks @dangotbanned! This looks great!

Would it be possible to add the examples that you describe within the docstring of the .transform_filter() method also within the data transformations in the official documentation around here: https://altair-viz.github.io/user_guide/transform/filter.html#logical-operands?

@dangotbanned
Copy link
Member Author

Thanks @dangotbanned! This looks great!

Would it be possible to add the examples that you describe within the docstring of the .transform_filter() method also within the data transformations in the official documentation around here: altair-viz.github.io/user_guide/transform/filter.html#logical-operands?

Thanks @mattijn!

Yeah that sounds like a good idea, will try and take a look later today

@dangotbanned
Copy link
Member Author

@mattijn preview from (3efc144)

I'm quite happy with the flow in the transform_filter() docstring.
But the other parts are already covered earlier in the page

image

@mattijn mattijn merged commit 1d576a8 into main Nov 13, 2024
25 checks passed
@mattijn
Copy link
Contributor

mattijn commented Nov 13, 2024

Thanks @dangotbanned 🥳!

@dangotbanned dangotbanned deleted the transform-filter-predicates branch November 13, 2024 21:42
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.

Update Chart.transform_filter() to mirror alt.when()
2 participants