Merge pull request #3049 from finos/granular-features

Better `features` support

Author: texodus

packages/perspective-jupyterlab/src/js/view.js

@@ -85,7 +85,7 @@ export class PerspectiveView extends DOMWidgetView {
                     });
                 }
             );
-            await this.perspective_client.init();
+
             const tableName = this.model.get("table_name");
             if (!tableName) throw new Error("table_name not set in model");
             const table = this.perspective_client

packages/perspective-viewer-datagrid/src/js/data_listener/format_tree_header.js

@@ -20,7 +20,11 @@ import { format_cell } from "./format_cell.js";
  * @param {*} row_headers
  * @param {*} regularTable
  */
-export function* format_tree_header(paths = [], row_headers, regularTable) {
+export function* format_tree_header_row_path(
+    paths = [],
+    row_headers,
+    regularTable
+) {
     const plugins = regularTable[PRIVATE_PLUGIN_SYMBOL];
     for (let path of paths) {
         path = ["TOTAL", ...path];
@@ -44,3 +48,23 @@ export function* format_tree_header(paths = [], row_headers, regularTable) {
         yield path;
     }
 }
+
+export function* format_tree_header(paths = [], row_headers, regularTable) {
+    const plugins = regularTable[PRIVATE_PLUGIN_SYMBOL];
+    for (let path of paths) {
+        const new_path = [""];
+        for (const idx in path) {
+            new_path.push(
+                format_cell.call(
+                    this,
+                    row_headers[idx],
+                    path[idx],
+                    plugins,
+                    true
+                )
+            );
+        }
+
+        yield path;
+    }
+}

packages/perspective-viewer-datagrid/src/js/data_listener/index.js

@@ -12,7 +12,10 @@
 
 import { PRIVATE_PLUGIN_SYMBOL } from "../model";
 import { format_cell } from "./format_cell.js";
-import { format_tree_header } from "./format_tree_header.js";
+import {
+    format_tree_header,
+    format_tree_header_row_path,
+} from "./format_tree_header.js";
 
 /**
  * Creates a new DataListener, suitable for passing to `regular-table`'s
@@ -86,15 +89,20 @@ export function createDataListener(viewer) {
             }
 
             this._last_window = new_window;
-            this._ids = columns.__ID__;
+            this._ids =
+                columns.__ID__ ||
+                Array(y1 - y0)
+                    .fill()
+                    .map((_, index) => [index + y0]);
+
             this._reverse_columns = this._column_paths
                 .slice(x0, x1)
                 .reduce((acc, x, i) => {
                     acc.set(x, i);
                     return acc;
                 }, new Map());
 
-            this._reverse_ids = this._ids.reduce((acc, x, i) => {
+            this._reverse_ids = this._ids?.reduce((acc, x, i) => {
                 acc.set(x?.join("|"), i);
                 return acc;
             }, new Map());
@@ -108,12 +116,6 @@ export function createDataListener(viewer) {
             column_paths = [];
 
         const is_settings_open = viewer.hasAttribute("settings");
-
-        // if (this._config.split_by?.length > 0) {
-        //     this._column_paths
-        // }
-
-        // for (const path of this._column_paths.slice(x0, x1)) {
         for (
             let ipath = x0;
             ipath < Math.min(x1, this._column_paths.length);
@@ -143,6 +145,7 @@ export function createDataListener(viewer) {
                     }
                 })
             );
+
             metadata.push(column);
             if (is_settings_open) {
                 path_parts.push("");
@@ -167,19 +170,27 @@ export function createDataListener(viewer) {
             last_reverse_columns = this._reverse_columns;
         }
 
-        return {
+        const is_row_path = columns.__ROW_PATH__ !== undefined;
+        const row_headers = Array.from(
+            (is_row_path
+                ? format_tree_header_row_path
+                : format_tree_header
+            ).call(
+                this,
+                columns.__ROW_PATH__,
+                this._config.group_by,
+                regularTable
+            )
+        );
+
+        const num_row_headers = row_headers[0]?.length;
+
+        const result = {
             num_column_headers: column_headers[0]?.length || 1,
-            num_row_headers: this._config.group_by.length + 1,
+            num_row_headers,
             num_rows: this._num_rows,
             num_columns: this._column_paths.length,
-            row_headers: Array.from(
-                format_tree_header.call(
-                    this,
-                    columns.__ROW_PATH__,
-                    this._config.group_by,
-                    regularTable
-                )
-            ),
+            row_headers,
             column_headers,
             data,
             metadata,
@@ -188,5 +199,7 @@ export function createDataListener(viewer) {
                 this._config.split_by.length
             ),
         };
+
+        return result;
     };
 }

packages/perspective-viewer-datagrid/src/js/event_handlers/row_select_click.js

@@ -29,7 +29,7 @@ export async function selectionListener(
         return;
     }
 
-    const id = this._ids[meta.y - meta.y0];
+    const id = this._ids?.[meta.y - meta.y0];
     if (meta && meta.y >= 0) {
         const selected = selected_rows_map.get(regularTable);
         const key_match =

packages/perspective-viewer-datagrid/src/js/model/create.js

@@ -116,26 +116,15 @@ export async function createModel(regular, table, view, extend = {}) {
             type_changed;
     }
 
-    // Extract the entire expression string as typed by the user, so we can
-    // feed it into `validate_expressions` and get back the data types for
-    // each column without it being affected by a pivot.
-    const expressions = Array.isArray(config.expressions)
-        ? Object.fromEntries(
-              config.expressions.map((expr) => [expr[0], expr[1]])
-          )
-        : config.expressions;
-
     const [
         table_schema,
-        validated_expressions,
         num_rows,
         schema,
         expression_schema,
         column_paths,
         _edit_port,
     ] = await Promise.all([
         table.schema(),
-        table.validate_expressions(expressions),
         view.num_rows(),
         view.schema(),
         view.expression_schema(),
@@ -170,7 +159,7 @@ export async function createModel(regular, table, view, extend = {}) {
     const _schema = { ...schema, ...expression_schema };
     const _table_schema = {
         ...table_schema,
-        ...validated_expressions.expression_schema,
+        ...expression_schema,
     };
 
     const _column_paths = column_paths.filter((path) => {

rust/perspective-client/perspective.proto

@@ -214,13 +214,26 @@ message GetFeaturesResp {
     bool group_by = 1;
     bool split_by = 2;
     bool expressions = 3;
-    map<uint32, ColumnTypeOptions>  filter_ops = 4;
+    bool on_update = 4;
+    bool sort = 5;
+    map<uint32, ColumnTypeOptions> filter_ops = 6;
+    map<uint32, AggregateOptions> aggregates = 7;
 
     message ColumnTypeOptions {
         repeated string options = 1;
     }
+
+    message AggregateOptions {
+        repeated AggregateArgs aggregates = 1;
+    }
+
+    message AggregateArgs {
+        string name = 1;
+        repeated ColumnType args = 2;
+    }
 }
 
+
 // `Client::get_hosted_tables`
 message GetHostedTablesReq {
     bool subscribe = 1;

rust/perspective-client/src/rust/client.rs

@@ -88,7 +88,7 @@ impl<U: Copy + 'static> SystemInfo<U> {
 
 /// Metadata about what features are supported by the `Server` to which this
 /// [`Client`] connects.
-#[derive(Clone, Default)]
+#[derive(Clone, Debug, Default)]
 pub struct Features(Arc<GetFeaturesResp>);
 
 impl Deref for Features {
@@ -376,21 +376,6 @@ impl Client {
         Ok(id)
     }
 
-    pub async fn init(&self) -> ClientResult<()> {
-        let msg = Request {
-            msg_id: self.gen_id(),
-            entity_id: "".to_owned(),
-            client_req: Some(ClientReq::GetFeaturesReq(GetFeaturesReq {})),
-        };
-
-        *self.features.lock().await = Some(Features(Arc::new(match self.oneshot(&msg).await? {
-            ClientResp::GetFeaturesResp(features) => Ok(features),
-            resp => Err(resp),
-        }?)));
-
-        Ok(())
-    }
-
     /// Generate a message ID unique to this client.
     pub(crate) fn gen_id(&self) -> u32 {
         self.id_gen.next()
@@ -461,14 +446,25 @@ impl Client {
             .map_err(|_| ClientError::Unknown(format!("Internal error for req {req}")))
     }
 
-    pub(crate) fn get_features(&self) -> ClientResult<Features> {
-        let features = self
-            .features
-            .try_lock()
-            .ok_or(ClientError::NotInitialized)?
-            .as_ref()
-            .ok_or(ClientError::NotInitialized)?
-            .clone();
+    pub(crate) async fn get_features(&self) -> ClientResult<Features> {
+        let mut guard = self.features.lock().await;
+        let features = if let Some(features) = &*guard {
+            features.clone()
+        } else {
+            let msg = Request {
+                msg_id: self.gen_id(),
+                entity_id: "".to_owned(),
+                client_req: Some(ClientReq::GetFeaturesReq(GetFeaturesReq {})),
+            };
+
+            let features = Features(Arc::new(match self.oneshot(&msg).await? {
+                ClientResp::GetFeaturesResp(features) => Ok(features),
+                resp => Err(resp),
+            }?));
+
+            *guard = Some(features.clone());
+            features
+        };
 
         Ok(features)
     }