Refactor: Distinguish between FieldPredicate and SelectionFilter #3275
Conversation
kanitw
force-pushed
the
kw/field-predicate
branch
from
567910e to
1e74028
Compare
site/docs/transform/filter.md
Outdated
| ### Selection Filter | ||
| ## Selection Filter | ||
|
|
||
| For a selection filter object, a [`selection` name](#selectionfilter) must be provided. |
site/docs/transform/filter.md
Outdated
| @@ -30,13 +30,12 @@ Vega-Lite filter transforms must have the `filter` property. | |||
| For a [Vega Expression](https://vega.github.io/vega/docs/expressions/) string, each datum object can be referred using bound variable `datum`. For example, setting `filter` to `"datum.b2 > 60"` would make the output data includes only items that have values in the field `b2` over 60. | |||
|
|
|||
|
|
|||
| ## Filter Object | |||
| ## Field Predicate Object | |||
There was a problem hiding this comment.
This is a bit inconsistent. Below, we call the filter "Selection Filter". I'd prefer to keep the name (it is also less confusing) and say that the "Filter Object" has to contain a "Filter Predicate". Alternatively, we could name it "Field Filter Object". The predicate is the expression that is used to filter.
There was a problem hiding this comment.
I will change selection filter to selection predicate. We should not call them filters because they are also use for condition syntax in a follow-up PR. Field predicate is also more applicable in more places so we need to distinguish them. The old text is confusing anyway.
kanitw
force-pushed
the
kw/field-predicate
branch
2 times, most recently
from
fe48c3e to
8cb65d6
Compare
src/filter.ts
Outdated
| @@ -9,25 +9,30 @@ import {isArray, isString, logicalExpr} from './util'; | |||
|
|
|||
|
|
|||
| export type Filter = | |||
There was a problem hiding this comment.
Actually Let me rename this whole thing to predicate
kanitw
force-pushed
the
kw/field-predicate
branch
from
8cb65d6 to
b8fec23
Compare
kanitw
force-pushed
the
kw/field-predicate
branch
from
b8fec23 to
cd2bb63
Compare
| */ | ||
| filter: LogicalOperand<Filter>; | ||
| // TODO: https://github.com/vega/vega-lite/issues/2901 |
There was a problem hiding this comment.
An hour or two -- in any case, it is NOT the scope of this PR. I'm just noting down something that was already missing. You're welcome to help contribute though.
| name: Filter | ||
| link: "transform.html#filter" | ||
| "LogicalOperand<Predicate>": | ||
| link: "transform.html#predicate" |
There was a problem hiding this comment.
do you want to give this a better name?
There was a problem hiding this comment.
This is good since there is LogicalOperand<String> too
So having generic should allow us to write about it only once
There was a problem hiding this comment.
I see. Go ahead and merge then.
(FieldPredicate can be useful in other scenario besides filter -- e.g., for condition in #3239 or for selecting data for annotation (@starry97's project)
cc: @starry97