From 6ae82825b63e80b9f4ba8dce2b7b074934c703d7 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Fri, 17 Nov 2023 02:16:25 -0500 Subject: [PATCH] Fix new expression syntax in Python --- packages/perspective/src/js/perspective.js | 14 +- .../perspective/src/js/perspective.node.js | 14 +- .../perspective/perspective/table/_utils.py | 142 ++++++++++++------ .../perspective/tests/manager/test_manager.py | 2 +- .../tests/table/test_view_expression.py | 22 +-- .../perspective/tests/viewer/test_validate.py | 2 +- .../perspective/tests/viewer/test_viewer.py | 8 +- .../perspective/viewer/validate.py | 5 + .../perspective/perspective/viewer/viewer.py | 4 +- .../perspective/viewer/viewer_traitlets.py | 2 +- .../src/less/column-selector.less | 1 + rust/perspective-viewer/src/ts/init.ts | 25 +-- rust/perspective-viewer/tasks/bundle/main.rs | 14 +- 13 files changed, 144 insertions(+), 111 deletions(-) diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js index fa5f1708cb..29710fdcd5 100644 --- a/packages/perspective/src/js/perspective.js +++ b/packages/perspective/src/js/perspective.js @@ -2208,14 +2208,12 @@ export default function (Module) { */ async init(msg) { let wasmBinary = msg.buffer; - try { - const mod = await WebAssembly.instantiate(wasmBinary); - const exports = mod.instance.exports; - const size = exports.size(); - const offset = exports.offset(); - const array = new Uint8Array(exports.memory.buffer); - wasmBinary = array.slice(offset, offset + size); - } catch {} + const mod = await WebAssembly.instantiate(wasmBinary); + const exports = mod.instance.exports; + const size = exports.size(); + const offset = exports.offset(); + const array = new Uint8Array(exports.memory.buffer); + wasmBinary = array.slice(offset, offset + size); __MODULE__ = await __MODULE__({ wasmBinary, wasmJSMethod: "native-wasm", diff --git a/packages/perspective/src/js/perspective.node.js b/packages/perspective/src/js/perspective.node.js index d8aa3e484b..cfaa16ddd4 100644 --- a/packages/perspective/src/js/perspective.node.js +++ b/packages/perspective/src/js/perspective.node.js @@ -34,14 +34,12 @@ const SYNC_SERVER = new (class extends Server { init(msg) { this._loaded_promise = (async () => { let wasmBinary = await buffer; - try { - const mod = await WebAssembly.instantiate(wasmBinary); - const exports = mod.instance.exports; - const size = exports.size(); - const offset = exports.offset(); - const array = new Uint8Array(exports.memory.buffer); - wasmBinary = array.slice(offset, offset + size); - } catch {} + const mod = await WebAssembly.instantiate(wasmBinary); + const exports = mod.instance.exports; + const size = exports.size(); + const offset = exports.offset(); + const array = new Uint8Array(exports.memory.buffer); + wasmBinary = array.slice(offset, offset + size); const core = await load_perspective({ wasmBinary, wasmJSMethod: "native-wasm", diff --git a/python/perspective/perspective/table/_utils.py b/python/perspective/perspective/table/_utils.py index 4304ab5b3e..889e66541b 100644 --- a/python/perspective/perspective/table/_utils.py +++ b/python/perspective/perspective/table/_utils.py @@ -10,7 +10,6 @@ # ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ # ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ -from operator import itemgetter import re from datetime import date, datetime from functools import partial @@ -156,11 +155,54 @@ def _parse_expression_inputs(expressions): validated_expressions = [] alias_map = {} - for expression in expressions: - if type(expression) is dict: - expr_raw, alias = itemgetter("expr", "name")(expression) - parsed = expr_raw - else: + if isinstance(expressions, dict): + for alias in expressions.keys(): + expression = expressions[alias] + + column_id_map = {} + column_name_map = {} + + # we need to be able to modify the running_cidx inside of every call to + # replacer_fn - must pass by reference unfortunately + running_cidx = [0] + + replacer_fn = partial( + _replace_expression_column_name, + column_name_map, + column_id_map, + running_cidx, + ) + + parsed = expression + parsed = re.sub( + BOOLEAN_LITERAL_REGEX, + lambda match: "True" if match.group(0) == "true" else ("False" if match.group(0) == "false" else match.group(0)), + parsed, + ) + + parsed = re.sub(EXPRESSION_COLUMN_NAME_REGEX, replacer_fn, parsed) + parsed = re.sub( + STRING_LITERAL_REGEX, + lambda match: "intern({0})".format(match.group(0)), + parsed, + ) + + # remove the `intern()` in bucket and regex functions that take + # string literal parameters. TODO this logic should be centralized + # in C++ instead of being duplicated. + parsed = re.sub(FUNCTION_LITERAL_REGEX, _replace_interned_param, parsed) + parsed = re.sub(REPLACE_FN_REGEX, _replace_interned_param, parsed) + + validated = [alias, expression, parsed, column_id_map] + + if alias_map.get(alias) is not None: + idx = alias_map[alias] + validated_expressions[idx] = validated + else: + validated_expressions.append(validated) + alias_map[alias] = len(validated_expressions) - 1 + if isinstance(expressions, list): + for expression in expressions: expr_raw = expression parsed = expr_raw alias_match = re.match(ALIAS_REGEX, expr_raw) @@ -169,49 +211,49 @@ def _parse_expression_inputs(expressions): else: alias = expr_raw - if '""' in expr_raw: - raise ValueError("Cannot reference empty column in expression!") - - column_id_map = {} - column_name_map = {} - - # we need to be able to modify the running_cidx inside of every call to - # replacer_fn - must pass by reference unfortunately - running_cidx = [0] - - replacer_fn = partial( - _replace_expression_column_name, - column_name_map, - column_id_map, - running_cidx, - ) - - parsed = re.sub( - BOOLEAN_LITERAL_REGEX, - lambda match: "True" if match.group(0) == "true" else ("False" if match.group(0) == "false" else match.group(0)), - parsed, - ) - - parsed = re.sub(EXPRESSION_COLUMN_NAME_REGEX, replacer_fn, parsed) - parsed = re.sub( - STRING_LITERAL_REGEX, - lambda match: "intern({0})".format(match.group(0)), - parsed, - ) - - # remove the `intern()` in bucket and regex functions that take - # string literal parameters. TODO this logic should be centralized - # in C++ instead of being duplicated. - parsed = re.sub(FUNCTION_LITERAL_REGEX, _replace_interned_param, parsed) - parsed = re.sub(REPLACE_FN_REGEX, _replace_interned_param, parsed) - - validated = [alias, expr_raw, parsed, column_id_map] - - if alias_map.get(alias) is not None: - idx = alias_map[alias] - validated_expressions[idx] = validated - else: - validated_expressions.append(validated) - alias_map[alias] = len(validated_expressions) - 1 + if '""' in expr_raw: + raise ValueError("Cannot reference empty column in expression!") + + column_id_map = {} + column_name_map = {} + + # we need to be able to modify the running_cidx inside of every call to + # replacer_fn - must pass by reference unfortunately + running_cidx = [0] + + replacer_fn = partial( + _replace_expression_column_name, + column_name_map, + column_id_map, + running_cidx, + ) + + parsed = re.sub( + BOOLEAN_LITERAL_REGEX, + lambda match: "True" if match.group(0) == "true" else ("False" if match.group(0) == "false" else match.group(0)), + parsed, + ) + + parsed = re.sub(EXPRESSION_COLUMN_NAME_REGEX, replacer_fn, parsed) + parsed = re.sub( + STRING_LITERAL_REGEX, + lambda match: "intern({0})".format(match.group(0)), + parsed, + ) + + # remove the `intern()` in bucket and regex functions that take + # string literal parameters. TODO this logic should be centralized + # in C++ instead of being duplicated. + parsed = re.sub(FUNCTION_LITERAL_REGEX, _replace_interned_param, parsed) + parsed = re.sub(REPLACE_FN_REGEX, _replace_interned_param, parsed) + + validated = [alias, expr_raw, parsed, column_id_map] + + if alias_map.get(alias) is not None: + idx = alias_map[alias] + validated_expressions[idx] = validated + else: + validated_expressions.append(validated) + alias_map[alias] = len(validated_expressions) - 1 return validated_expressions diff --git a/python/perspective/perspective/tests/manager/test_manager.py b/python/perspective/perspective/tests/manager/test_manager.py index bff022bd16..78b66bf514 100644 --- a/python/perspective/perspective/tests/manager/test_manager.py +++ b/python/perspective/perspective/tests/manager/test_manager.py @@ -471,7 +471,7 @@ def test_manager_view_expression_schema(self): "table_name": "table1", "view_name": "view1", "cmd": "view", - "config": {"expressions": ['// abc \n "a" + "a"']}, + "config": {"expressions": {"abc": '"a" + "a"'}}, } message = { "id": 2, diff --git a/python/perspective/perspective/tests/table/test_view_expression.py b/python/perspective/perspective/tests/table/test_view_expression.py index 8aa67cfc46..21488b56c8 100644 --- a/python/perspective/perspective/tests/table/test_view_expression.py +++ b/python/perspective/perspective/tests/table/test_view_expression.py @@ -37,16 +37,18 @@ def test_expression_conversion(self): "// g\nnow()", "// h\nlength(123)", ] - test_expressions_dict = [ - {"name": "x", "expr": '"a"'}, - {"name": "y", "expr": '"b" * 0.5'}, - {"name": "c", "expr": "'abcdefg'"}, - {"name": "d", "expr": "true and false"}, - {"name": "e", "expr": 'float("a") > 2 ? null : 1'}, - {"name": "f", "expr": "today()"}, - {"name": "g", "expr": "now()"}, - {"name": "h", "expr": "length(123)"}, - ] + + test_expressions_dict = { + "x": '"a"', + "y": '"b" * 0.5', + "c": "'abcdefg'", + "d": "true and false", + "e": 'float("a") > 2 ? null : 1', + "f": "today()", + "g": "now()", + "h": "length(123)", + } + str_validated = table.validate_expressions(test_expressions_str) dict_validated = table.validate_expressions(test_expressions_dict) assert str_validated["errors"] == dict_validated["errors"] diff --git a/python/perspective/perspective/tests/viewer/test_validate.py b/python/perspective/perspective/tests/viewer/test_validate.py index 5edc23ec48..a227cdeb57 100644 --- a/python/perspective/perspective/tests/viewer/test_validate.py +++ b/python/perspective/perspective/tests/viewer/test_validate.py @@ -57,4 +57,4 @@ def test_validate_expressions(self): def test_validate_expressions_invalid(self): with raises(PerspectiveError): - assert validate.validate_expressions({}) + assert validate.validate_expressions([]) diff --git a/python/perspective/perspective/tests/viewer/test_viewer.py b/python/perspective/perspective/tests/viewer/test_viewer.py index 8ba4b6ad36..72b9ab6a31 100644 --- a/python/perspective/perspective/tests/viewer/test_viewer.py +++ b/python/perspective/perspective/tests/viewer/test_viewer.py @@ -198,7 +198,7 @@ def test_viewer_delete_without_table(self): def test_save_restore(self): table = Table({"a": [1, 2, 3]}) - viewer = PerspectiveViewer(plugin="X Bar", filter=[["a", "==", 2]], expressions=['"a" * 2']) + viewer = PerspectiveViewer(plugin="X Bar", filter=[["a", "==", 2]], expressions={'"a" * 2': '"a" * 2'}) viewer.load(table) # Save config @@ -207,19 +207,19 @@ def test_save_restore(self): assert config["filter"] == [["a", "==", 2]] assert viewer.plugin == "X Bar" assert config["plugin"] == "X Bar" - assert config["expressions"] == ['"a" * 2'] + assert config["expressions"] == {'"a" * 2': '"a" * 2'} # reset configuration viewer.reset() assert viewer.plugin == "Datagrid" assert viewer.filter == [] - assert viewer.expressions == [] + assert viewer.expressions == {} # restore configuration viewer.restore(**config) assert viewer.filter == [["a", "==", 2]] assert viewer.plugin == "X Bar" - assert viewer.expressions == ['"a" * 2'] + assert viewer.expressions == {'"a" * 2': '"a" * 2'} def test_save_restore_plugin_config(self): viewer = PerspectiveViewer(plugin="Datagrid", plugin_config={"columns": {"a": {"fixed": 4}}}) diff --git a/python/perspective/perspective/viewer/validate.py b/python/perspective/perspective/viewer/validate.py index cd9ec84bdf..1cc4d7a8c7 100644 --- a/python/perspective/perspective/viewer/validate.py +++ b/python/perspective/perspective/viewer/validate.py @@ -148,6 +148,11 @@ def validate_expressions(expressions): if not isinstance(expr, str): raise PerspectiveError("Cannot parse non-string expression: {}".format(str(type(expr)))) return expressions + elif isinstance(expressions, dict): + for expr in expressions.values(): + if not isinstance(expr, str): + raise PerspectiveError("Cannot parse non-string expression: {}".format(str(type(expr)))) + return expressions else: raise PerspectiveError("Cannot parse expressions of type: {}".format(str(type(expressions)))) diff --git a/python/perspective/perspective/viewer/viewer.py b/python/perspective/perspective/viewer/viewer.py index f05dcc1dde..6bf0d6e870 100644 --- a/python/perspective/perspective/viewer/viewer.py +++ b/python/perspective/perspective/viewer/viewer.py @@ -134,7 +134,7 @@ def __init__( self.aggregates = validate_aggregates(aggregates) or {} self.sort = validate_sort(sort) or [] self.filter = validate_filter(filter) or [] - self.expressions = validate_expressions(expressions) or [] + self.expressions = validate_expressions(expressions) or {} self.plugin_config = validate_plugin_config(plugin_config) or {} self.settings = settings self.theme = theme @@ -269,7 +269,7 @@ def reset(self): self.split_by = [] self.filter = [] self.sort = [] - self.expressions = [] + self.expressions = {} self.aggregates = {} self.columns = [] self.plugin = "Datagrid" diff --git a/python/perspective/perspective/viewer/viewer_traitlets.py b/python/perspective/perspective/viewer/viewer_traitlets.py index 7a60a6d42a..0b2c9a4bff 100644 --- a/python/perspective/perspective/viewer/viewer_traitlets.py +++ b/python/perspective/perspective/viewer/viewer_traitlets.py @@ -47,7 +47,7 @@ class PerspectiveTraitlets(HasTraits): aggregates = Dict(default_value={}).tag(sync=True) sort = List(default_value=[]).tag(sync=True) filter = List(default_value=[]).tag(sync=True) - expressions = List(default_value=[]).tag(sync=True) + expressions = Dict(default_value=[]).tag(sync=True) plugin_config = Dict(default_value={}).tag(sync=True) settings = Bool(True).tag(sync=True) theme = Unicode("Pro Light", allow_none=True).tag(sync=True) diff --git a/rust/perspective-viewer/src/less/column-selector.less b/rust/perspective-viewer/src/less/column-selector.less index 6887f240f0..4ba525c316 100644 --- a/rust/perspective-viewer/src/less/column-selector.less +++ b/rust/perspective-viewer/src/less/column-selector.less @@ -132,6 +132,7 @@ align-items: center; margin-right: 7px; cursor: pointer; + margin-bottom: 4px; // Button icon &:before { diff --git a/rust/perspective-viewer/src/ts/init.ts b/rust/perspective-viewer/src/ts/init.ts index b268263bcb..d87a47498f 100644 --- a/rust/perspective-viewer/src/ts/init.ts +++ b/rust/perspective-viewer/src/ts/init.ts @@ -30,15 +30,19 @@ type Module = { memory: WebAssembly.Memory; }; +// Perform a silly dance to deal with the different ways webpack and esbuild +// load binary, as this may either be an `ArrayBuffer` or `URL` depening +// on whether `inline` option was specified to `perspective-esbuild-plugin`. async function compile(buff_or_url) { - if (buff_or_url instanceof URL) { + if (buff_or_url instanceof URL || typeof buff_or_url == "string") { return await WebAssembly.instantiateStreaming(fetch(buff_or_url)); } else { return await WebAssembly.instantiate(buff_or_url); } } -async function release_build(buff_or_url) { +async function load_wasm() { + const buff_or_url = await wasm; const mod = await compile(buff_or_url); const exports = mod.instance.exports as Module; const size = exports.size(); @@ -46,23 +50,6 @@ async function release_build(buff_or_url) { const array = new Uint8Array(exports.memory.buffer); const uncompressed_wasm = array.slice(offset, offset + size); await wasm_module.default(uncompressed_wasm); -} - -async function debug_build(buff_or_url) { - await wasm_module.default(buff_or_url); -} - -async function load_wasm() { - // Perform a silly dance to deal with the different ways webpack and esbuild - // load binary, as this may either be an `ArrayBuffer` or `URL` depening - // on whether `inline` option was specified to `perspective-esbuild-plugin`. - const buff_or_url = await wasm; - try { - await release_build(buff_or_url); - } catch { - await debug_build(buff_or_url); - } - wasm_module.init(); return wasm_module; } diff --git a/rust/perspective-viewer/tasks/bundle/main.rs b/rust/perspective-viewer/tasks/bundle/main.rs index 5b3dd3b1d1..7826cf79c1 100644 --- a/rust/perspective-viewer/tasks/bundle/main.rs +++ b/rust/perspective-viewer/tasks/bundle/main.rs @@ -70,14 +70,14 @@ fn opt(outpath: &Path) { .one_caller_inline_max_size(15) .run(outpath, outpath) .unwrap(); - - Command::new("cargo") - .args(["run"]) - .args(["-p", "perspective-bootstrap"]) - .args(["--"]) - .args(["dist/pkg/perspective_bg.wasm"]) - .execute(); } + + Command::new("cargo") + .args(["run"]) + .args(["-p", "perspective-bootstrap"]) + .args(["--"]) + .args(["dist/pkg/perspective_bg.wasm"]) + .execute(); } fn main() {