Add filter for valid fields. Fixes #4098 #4109
Conversation
src/predicate.ts
Outdated
| @@ -139,12 +141,20 @@ export interface FieldOneOfPredicate extends FieldPredicateBase { | |||
| oneOf: string[] | number[] | boolean[] | DateTime[]; | |||
| } | |||
|
|
|||
| export interface FieldValidPredicate extends FieldPredicateBase { | |||
| valid: true; | |||
There was a problem hiding this comment.
This should support false too. Users may want to analyze the pattern of data that have some fields missing.
There was a problem hiding this comment.
You can always use not. I'd rather support fewer ways to do the same thing for now. We can change it later.
There was a problem hiding this comment.
I disagree. Users wouldn't expect that it can't be false, so better fix it now since it's very easy and provides a way nicer syntax that nesting it under "not".
site/docs/transform/filter.md
Outdated
|
|
||
| {% include table.html props="valid" source="FieldValidPredicate" %} | ||
|
|
||
| For example, `{"filter": {"field": "car_color", "valid": true}}` checks if the `car_color` field's value is not `null` and whether it is not `NaN`. |
There was a problem hiding this comment.
Adjust the language to make it clear which data items would be included.
domoritz
force-pushed
the
dom/filter-valid
branch
from
e53d72e
to
2881fbc
Compare
|
Done |
domoritz
force-pushed
the
dom/filter-valid
branch
from
748f8e7
to
30a713f
Compare
|
I added some tests |
domoritz
force-pushed
the
dom/filter-valid
branch
from
35118da
to
7e71d53
Compare
|
100% patch coverage |
| return 'indexof([' + predicateValuesExpr(predicate.oneOf, timeUnit).join(',') + '], ' + fieldExpr + ') !== -1'; | ||
| return `indexof([${predicateValuesExpr(predicate.oneOf, timeUnit).join(',')}], ${fieldExpr}) !== -1`; | ||
| } else if (isFieldValidPredicate(predicate)) { | ||
| return predicate.valid ? `${fieldExpr}!==null&&!isNaN(${fieldExpr})` : `${fieldExpr}===null||isNaN(${fieldExpr})`; |
There was a problem hiding this comment.
I'd prefer having spaces for readability.
It might be worth submitting another PR to standardize other the filter expressions too.
(Currently, some do have spaces between operators, some do not.)
There was a problem hiding this comment.
Let's do this in a follow up. Issue: #4112
No description provided.