Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Improve filtering type awareness #410

Merged
merged 34 commits into from
Apr 25, 2019
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
ff43892
- generic configuration for lexer
Apr 17, 2019
8d5c5ad
- configurable syntax tree
Apr 17, 2019
0e73b9a
type awareness in single column query
Apr 17, 2019
92c744c
add standalone and unit tests for `contains` vs `eq` by-type behavior
Apr 17, 2019
4406df4
clean up
Apr 17, 2019
1089f86
- likeDate relational operator
Apr 23, 2019
eccf344
contains usable by all column types
Apr 24, 2019
6ff91fa
- update single column config
Apr 24, 2019
b624c37
fix e2e test (legacy impl)
Apr 24, 2019
aeac7fc
update ci test breakdown
Apr 24, 2019
32ac750
py env for py tests
Apr 24, 2019
eb9ece4
unit tests venv
Apr 24, 2019
2ec2812
update standalone tests
Apr 24, 2019
d58611c
allow cross-type `eq` comparison
Apr 24, 2019
17482ee
- add 'in' (like date) operator
Apr 24, 2019
67dcdb3
- reduce cypress memory usage (ts mappings)
Apr 24, 2019
5980714
- revert unused changes
Apr 24, 2019
91a16cb
revert generic lexicon changes
Apr 24, 2019
f95cda9
single column `any` test update
Apr 24, 2019
4e93186
clean up
Apr 25, 2019
0d730cc
simple column query config rework
Apr 25, 2019
4e3fe5f
single column query config rework
Apr 25, 2019
297706b
rename `in` -> `like`
Apr 25, 2019
606a7c6
- remove `operand` in favor of `fieldExpression`
Apr 25, 2019
6ad4882
Merge branch 'master' into improve-filter-type-awareness
Marc-Andre-Rivet Apr 25, 2019
3f6fd78
- fix expression terminal logic
Apr 25, 2019
79f551d
Merge branch 'improve-filter-type-awareness' of github.com:plotly/das…
Apr 25, 2019
b276e53
filter out empty column filters
Apr 25, 2019
e51f7ec
rework
Apr 25, 2019
350b5e4
revert filterfactory change
Apr 25, 2019
d905551
- reowrk `contains` and `eq` logic
Apr 25, 2019
72116ea
- operand / expression regression
Apr 25, 2019
7971d76
rename `like` relational operator to `dateStartsWith`
Apr 25, 2019
9fda146
update changelog
Apr 25, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 71 additions & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
version: 2

jobs:
"test":
"server-test":
docker:
- image: circleci/python:3.6.7-node-browsers
- image: cypress/base:10
Expand Down Expand Up @@ -44,7 +44,73 @@ jobs:
name: Run tests
command: |
. venv/bin/activate
npm run test
npm run test.server


"standalone-test":
docker:
- image: circleci/python:3.6.7-node-browsers
- image: cypress/base:10

steps:
- checkout
- restore_cache:
key: deps1-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
- run:
name: Install npm packages
command: npm install
- run:
name: Cypress Install
command: |
$(npm bin)/cypress install

- save_cache:
key: deps1-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
paths:
- node_modules
- /home/circleci/.cache/Cypress

- run:
name: Run tests
command: npm run test.standalone


"unit-test":
docker:
- image: circleci/python:3.6.7-node-browsers
- image: cypress/base:10

steps:
- checkout
- restore_cache:
key: deps1-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
- run:
name: Install npm packages
command: npm install
- run:
name: Cypress Install
command: |
$(npm bin)/cypress install

- save_cache:
key: deps1-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
paths:
- node_modules
- /home/circleci/.cache/Cypress

- run:
name: Install requirements
command: |
sudo pip install --upgrade virtualenv
python -m venv venv || virtualenv venv
. venv/bin/activate
pip install -r requirements.txt --quiet

- run:
name: Run tests
command: |
. venv/bin/activate
npm run test.unit


"visual-test":
Expand Down Expand Up @@ -170,5 +236,7 @@ workflows:
jobs:
- "python-3.6"
- "node"
- "test"
- "server-test"
- "standalone-test"
- "unit-test"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were taking way too long. Breaking up the server, standalone and unit tests.

- "visual-test"
21 changes: 12 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
"main": "dash_table/bundle.js",
"scripts": {
"preprivate::opentests": "run-s private::wait*",
"preprivate::runtests": "run-s private::wait*",
"preprivate::test.server": "run-s private::wait_dash*",
"preprivate::test.standalone": "run-s private::wait_js",
"pretest.standalone": "run-s private::build:js-test",
"private::build": "node --max_old_space_size=4096 node_modules/webpack/bin/webpack --display-reasons --bail",
"private::build:js": "run-s \"private::build -- --mode production\"",
"private::build:js-dev": "run-s \"private::build -- --mode development\"",
Expand All @@ -26,15 +28,16 @@
"private::wait_dash8083": "wait-on http://localhost:8083",
"private::wait_js": "wait-on http://localhost:8080",
"private::opentests": "cypress open",
"private::runtests:python": "python -m unittest tests/unit/format_test.py",
"private::runtests:unit": "cypress run --browser chrome --spec 'tests/cypress/tests/unit/**/*'",
"private::runtests:standalone": "cypress run --browser chrome --spec 'tests/cypress/tests/standalone/**/*'",
"private::runtests:server": "cypress run --browser chrome --spec 'tests/cypress/tests/server/**/*'",
"private::runtests": "run-s private::runtests:python private::runtests:unit private::runtests:standalone private::runtests:server",
"private::test.python": "python -m unittest tests/unit/format_test.py",
"private::test.unit": "cypress run --browser chrome --spec 'tests/cypress/tests/unit/**/*'",
"private::test.server": "cypress run --browser chrome --spec 'tests/cypress/tests/server/**/*'",
"private::test.standalone": "cypress run --browser chrome --spec 'tests/cypress/tests/standalone/**/*'",
"build.watch": "webpack-dev-server --content-base dash_table --mode development",
"build": "run-s private::build:js private::build:py",
"lint": "run-s private::lint:*",
"test": "run-p --race private::host* private::runtests",
"test.server": "run-p --race private::host* private::test.server",
"test.standalone": "run-p --race private::host_js private::test.standalone",
"test.unit": "run-s private::test.python private::test.unit",
"test.visual": "build-storybook && percy-storybook",
"test.visual-local": "build-storybook",
"test.watch": "run-p --race \"private::build:js-test-watch\" --race private::host* private::opentests"
Expand All @@ -54,7 +57,7 @@
"@storybook/react": "^5.0.5",
"@types/d3-format": "^1.3.1",
"@types/papaparse": "^4.5.9",
"@types/ramda": "^0.26.5",
"@types/ramda": "^0.26.6",
"@types/react": "^16.8.8",
"@types/react-dom": "^16.8.3",
"@types/react-select": "^1.3.4",
Expand Down Expand Up @@ -82,7 +85,7 @@
"style-loader": "^0.23.1",
"ts-loader": "^5.3.3",
"tslint": "^5.14.0",
"typescript": "^3.3.4000",
"typescript": "^3.4.3",
"wait-on": "^3.2.0",
"webpack": "^4.29.6",
"webpack-cli": "^3.3.0",
Expand Down
7 changes: 5 additions & 2 deletions src/core/syntax-tree/lexicon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ export interface IUnboundedLexeme {
syntaxer?: (lexs: any[], pivot: any, pivotIndex: number) => any;
}

type If = (string | undefined)[] | ((lexemes: ILexemeResult[], previous: ILexemeResult) => boolean);
type Terminal = boolean | ((lexemes: ILexemeResult[], previous: ILexemeResult) => boolean);

export interface ILexeme extends IUnboundedLexeme {
terminal: boolean | ((lexemes: ILexemeResult[], previous: ILexemeResult) => boolean);
if: (string | undefined)[] | ((lexemes: ILexemeResult[], previous: ILexemeResult) => boolean);
if: If;
terminal: Terminal;
}

export function boundLexeme(lexeme: IUnboundedLexeme) {
Expand Down
28 changes: 14 additions & 14 deletions src/dash-table/components/FilterFactory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ export default class FilterFactory {

}

private onChange = (columnId: ColumnId, setFilter: SetFilter, ev: any) => {
Logger.debug('Filter -- onChange', columnId, ev.target.value && ev.target.value.trim());
private onChange = (column: IVisibleColumn, setFilter: SetFilter, ev: any) => {
Logger.debug('Filter -- onChange', column.id, ev.target.value && ev.target.value.trim());

const value = ev.target.value.trim();
const safeColumnId = columnId.toString();
const safeColumnId = column.id.toString();

if (value && value.length) {
this.ops.set(safeColumnId, new SingleColumnSyntaxTree(safeColumnId, value));
this.ops.set(safeColumnId, new SingleColumnSyntaxTree(value, { column }));
} else {
this.ops.delete(safeColumnId);
}
Expand All @@ -65,26 +65,26 @@ export default class FilterFactory {

const rawGlobalFilter = R.map(
ast => ast.query || '',
R.filter(ast => Boolean(ast), asts)
R.filter<SingleColumnSyntaxTree>(ast => Boolean(ast), asts)
).join(' && ');

setFilter(globalFilter, rawGlobalFilter);
}

private getEventHandler = (fn: Function, columnId: ColumnId, setFilter: SetFilter): any => {
private getEventHandler = (fn: Function, column: IVisibleColumn, setFilter: SetFilter): any => {
const fnHandler = (this.handlers.get(fn) || this.handlers.set(fn, new Map()).get(fn));
const columnIdHandler = (fnHandler.get(columnId) || fnHandler.set(columnId, new Map()).get(columnId));
const columnIdHandler = (fnHandler.get(column.id) || fnHandler.set(column.id, new Map()).get(column.id));

return (
columnIdHandler.get(setFilter) ||
(columnIdHandler.set(setFilter, fn.bind(this, columnId, setFilter)).get(setFilter))
(columnIdHandler.set(setFilter, fn.bind(this, column, setFilter)).get(setFilter))
);
}

private updateOps = memoizeOne((query: string) => {
private updateOps = memoizeOne((query: string, columns: IVisibleColumn[]) => {
const multiQuery = new MultiColumnsSyntaxTree(query);

const newOps = getSingleColumnMap(multiQuery);
const newOps = getSingleColumnMap(multiQuery, columns);
if (!newOps) {
return;
}
Expand All @@ -109,15 +109,15 @@ export default class FilterFactory {
});

private filter = memoizerCache<[ColumnId, number]>()((
column: ColumnId,
column: IVisibleColumn,
index: number,
ast: SingleColumnSyntaxTree | undefined,
setFilter: SetFilter
) => {
return (<ColumnFilter
key={`column-${index}`}
classes={`dash-filter column-${index}`}
columnId={column}
columnId={column.id}
isValid={!ast || ast.isValid}
setFilter={this.getEventHandler(this.onChange, column, setFilter)}
value={ast && ast.query}
Expand All @@ -143,7 +143,7 @@ export default class FilterFactory {
return [];
}

this.updateOps(filter);
this.updateOps(filter, columns);

if (filtering_type === FilteringType.Basic) {
const filterStyles = this.relevantStyles(
Expand All @@ -160,7 +160,7 @@ export default class FilterFactory {

const filters = R.addIndex<IVisibleColumn, JSX.Element>(R.map)((column, index) => {
return this.filter.get(column.id, index)(
column.id,
column,
index,
this.ops.get(column.id.toString()),
setFilter
Expand Down
2 changes: 1 addition & 1 deletion src/dash-table/derived/cell/dropdowns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class Dropdowns {
...(staticDropdown ? [staticDropdown] : []),
...R.map(
([cd]) => cd.dropdown,
R.filter(
R.filter<[IConditionalDropdown, number]>(
([cd, i]) => this.evaluation.get(column.id, i)(
this.ast.get(column.id, i)(cd.condition),
datum
Expand Down
2 changes: 1 addition & 1 deletion src/dash-table/derived/cell/wrapperStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function getter(
return R.addIndex<any, Style[]>(R.map)((datum, index) => R.map(column => {
const relevantStyles = R.map(
s => s.style,
R.filter(
R.filter<IConvertedStyle>(
style =>
style.matchesColumn(column) &&
style.matchesRow(index + offset.rows) &&
Expand Down
2 changes: 1 addition & 1 deletion src/dash-table/derived/filter/wrapperStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function getter(
return R.map(column => {
const relevantStyles = R.map(
s => s.style,
R.filter(
R.filter<IConvertedStyle>(
style => style.matchesColumn(column),
filterStyles
)
Expand Down
2 changes: 1 addition & 1 deletion src/dash-table/derived/header/wrapperStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function getter(
return R.map(idx => R.map(column => {
const relevantStyles = R.map(
s => s.style,
R.filter(
R.filter<IConvertedStyle>(
style =>
style.matchesColumn(column) &&
style.matchesRow(idx),
Expand Down
9 changes: 8 additions & 1 deletion src/dash-table/syntax-tree/MultiColumnsSyntaxTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { LexemeType } from 'core/syntax-tree/lexicon';
import { ISyntaxTree } from 'core/syntax-tree/syntaxer';

import columnMultiLexicon from './lexicon/columnMulti';
import { ILexemeResult } from 'core/syntax-tree/lexer';

export default class MultiColumnsSyntaxTree extends SyntaxTree {
constructor(query: string) {
Expand Down Expand Up @@ -39,7 +40,13 @@ export default class MultiColumnsSyntaxTree extends SyntaxTree {
return statements;
}
private respectsBasicSyntax() {
const fields = R.map(item => item.value, R.filter(i => i.lexeme.type === LexemeType.Operand, this.lexerResult.lexemes));
const fields = R.map(
(item: ILexemeResult) => item.value,
R.filter(
i => i.lexeme.type === LexemeType.Operand,
this.lexerResult.lexemes
)
);
const uniqueFields = R.uniq(fields);
return fields.length === uniqueFields.length;
}
Expand Down
37 changes: 28 additions & 9 deletions src/dash-table/syntax-tree/SingleColumnSyntaxTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,24 @@ import SyntaxTree from 'core/syntax-tree';
import { ILexemeResult, ILexerResult } from 'core/syntax-tree/lexer';
import { LexemeType, boundLexeme } from 'core/syntax-tree/lexicon';

import { ColumnId } from 'dash-table/components/Table/props';
import { ColumnType } from 'dash-table/components/Table/props';

import operand from './lexeme/operand';
import { equal } from './lexeme/relational';
import { equal, RelationalOperator } from './lexeme/relational';

import columnLexicon from './lexicon/column';
import columnLexicon, { ISingleColumnConfig } from './lexicon/column';

function getDefaultRelationalOperator(type: ColumnType = ColumnType.Any): RelationalOperator {
switch (type) {
case ColumnType.Any:
case ColumnType.Text:
return RelationalOperator.Contains;
case ColumnType.Datetime:
return RelationalOperator.LikeDate;
case ColumnType.Numeric:
return RelationalOperator.Equal;
}
}

function isBinary(lexemes: ILexemeResult[]) {
return lexemes.length === 2;
Expand All @@ -23,20 +35,23 @@ function isUnary(lexemes: ILexemeResult[]) {
lexemes[0].lexeme.type === LexemeType.UnaryOperator;
}

export function modifyLex(key: ColumnId, res: ILexerResult) {
export function modifyLex(config: ISingleColumnConfig, res: ILexerResult) {
if (!res.valid) {
return res;
}

if (isBinary(res.lexemes) || isUnary(res.lexemes)) {
res.lexemes = [
{ lexeme: boundLexeme(operand), value: `{${key}}` },
{ lexeme: boundLexeme(operand), value: `{${config.column.id}}` },
...res.lexemes
];
} else if (isExpression(res.lexemes)) {
res.lexemes = [
{ lexeme: boundLexeme(operand), value: `{${key}}` },
{ lexeme: boundLexeme(equal), value: 'eq' },
{ lexeme: boundLexeme(operand), value: `{${config.column.id}}` },
{
lexeme: boundLexeme(equal),
value: getDefaultRelationalOperator(config.column.type)
},
...res.lexemes
];
}
Expand All @@ -45,7 +60,11 @@ export function modifyLex(key: ColumnId, res: ILexerResult) {
}

export default class SingleColumnSyntaxTree extends SyntaxTree {
constructor(key: ColumnId, query: string) {
super(columnLexicon, query, modifyLex.bind(undefined, key));
constructor(query: string, config: ISingleColumnConfig) {
super(
columnLexicon,
query,
modifyLex.bind(undefined, config)
);
}
}
Loading