From 765e275d442484276b0d2d6b105f11ff642d9d4f Mon Sep 17 00:00:00 2001 From: ada mandala Date: Wed, 15 Nov 2023 12:34:13 -0600 Subject: [PATCH] simplify expression serialization --- .../test/jupyter/widget.spec.js | 6 +- .../src/rust/config/expressions.rs | 132 +++++++----------- rust/perspective-viewer/src/rust/session.rs | 11 +- .../rust/session/replace_expression_update.rs | 6 +- 4 files changed, 61 insertions(+), 94 deletions(-) diff --git a/packages/perspective-jupyterlab/test/jupyter/widget.spec.js b/packages/perspective-jupyterlab/test/jupyter/widget.spec.js index 47ffc651bc..25ec83e832 100644 --- a/packages/perspective-jupyterlab/test/jupyter/widget.spec.js +++ b/packages/perspective-jupyterlab/test/jupyter/widget.spec.js @@ -232,7 +232,7 @@ describe_jupyter( "date", "datetime", ], - expressions: [], + expressions: {}, filter: [], group_by: [], plugin: "Datagrid", @@ -269,7 +269,7 @@ w.theme = "Pro Dark"` expect(config).toEqual({ aggregates: {}, columns: ["ui8"], - expressions: [], + expressions: {}, filter: [["i8", "<", 50]], group_by: ["date"], plugin: "X Bar", @@ -317,7 +317,7 @@ w.theme = "Pro Dark"` "date", "datetime", ], - expressions: [], + expressions: {}, filter: [], group_by: [], plugin: "Datagrid", diff --git a/rust/perspective-viewer/src/rust/config/expressions.rs b/rust/perspective-viewer/src/rust/config/expressions.rs index 9cd29987d1..96bebc3b09 100644 --- a/rust/perspective-viewer/src/rust/config/expressions.rs +++ b/rust/perspective-viewer/src/rust/config/expressions.rs @@ -13,110 +13,84 @@ use std::borrow::Cow; use std::collections::HashMap; -use itertools::Itertools; use serde::{Deserialize, Serialize}; -#[derive(Serialize, Deserialize, Clone, PartialEq, Debug)] +#[derive(Deserialize, Clone, PartialEq, Debug)] #[serde(untagged)] -pub enum Expressions { +pub enum ExpressionsDeserde { Array(Vec), Map(HashMap), } -#[derive(Clone, Debug, PartialEq)] -pub struct Expression<'a> { - pub name: Cow<'a, str>, - pub expression: Cow<'a, str>, -} +#[derive(Deserialize, Serialize, Clone, PartialEq, Debug, Default)] +#[serde(from = "ExpressionsDeserde")] +pub struct Expressions(HashMap); + +impl std::ops::Deref for Expressions { + type Target = HashMap; -impl Default for Expressions { - fn default() -> Self { - Self::Map(HashMap::default()) + fn deref(&self) -> &Self::Target { + &self.0 } } - -impl<'a> FromIterator> for Expressions { - fn from_iter>>(iter: T) -> Self { - Self::Map( - iter.into_iter() - .map(|x| (x.name.as_ref().to_owned(), x.expression.as_ref().to_owned())) - .collect(), - ) +impl std::ops::DerefMut for Expressions { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 } } -fn change(expressions: &[String]) -> HashMap { - expressions.iter().map(|s| { +impl From for Expressions { + fn from(value: ExpressionsDeserde) -> Self { + let change = |expressions: &[String]| { + expressions.iter().map(|s| { if let Some((name, expression)) = s.split_once('\n') && !expression.is_empty() && name.starts_with("//") { (name.split_at(2).1.trim().to_owned(), expression.to_owned()) } else { - (s.to_owned(), s.to_owned()) + // name = first 25 chars of the expr + let name = match s.char_indices().nth(25) { + None => s.to_owned(), + Some((idx, _)) => s[..idx].to_owned(), + }; + (name, s.to_owned()) } }).collect::>() -} - -impl Expressions { - #[must_use] - pub fn len(&self) -> usize { - match self { - Expressions::Array(x) => x.len(), - Expressions::Map(x) => x.len(), + }; + match value { + ExpressionsDeserde::Array(arr) => Self(change(&arr)), + ExpressionsDeserde::Map(map) => Self(map), } } +} - #[must_use] - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - pub fn remove(&mut self, expr: &str) { - match self { - Expressions::Array(expressions) => { - let mut map = change(expressions); - map.remove(expr); - *self = Self::Map(map); - } - Expressions::Map(map) => { - map.remove(expr); - } +#[derive(Clone, Debug, PartialEq)] +pub struct Expression<'a> { + pub name: Cow<'a, str>, + pub expression: Cow<'a, str>, +} +impl<'a> Expression<'a> { + pub fn new(name: &'a str, expression: &'a str) -> Self { + Self { + name: name.into(), + expression: expression.into(), } } +} - pub fn insert(&mut self, expr: &Expression) { - match self { - Expressions::Array(expressions) => { - let mut map = change(expressions); - map.insert( - expr.name.as_ref().to_owned(), - expr.expression.as_ref().to_owned(), - ); - *self = Self::Map(map); - } - Expressions::Map(map) => { - map.insert( - expr.name.as_ref().to_owned(), - expr.expression.as_ref().to_owned(), - ); - } - } +impl<'a> FromIterator> for Expressions { + fn from_iter>>(iter: T) -> Self { + Self( + iter.into_iter() + .map(|x| (x.name.as_ref().to_owned(), x.expression.as_ref().to_owned())) + .collect(), + ) } +} - pub fn iter(&self) -> Box> + '_> { - match self { - Expressions::Array(expressions) => { - Box::new(expressions.iter().map(|s| { - if let Some( (name, expression) ) = s.split_once('\n') && !expression.is_empty() && name.starts_with("//") { - Expression{ name: name.split_at(2).1.trim().into(), expression: expression.into() } - } else { - Expression{ name: s.into(), expression: s.into() } - } - })) - } - Expressions::Map(map) => { - Box::new(map.keys().sorted().map(|x| { - Expression { name: x.into(), expression: map.get(x).unwrap().into() } - })) - }, - } +impl Expressions { + pub fn insert(&mut self, expr: &Expression) { + self.0.insert( + expr.name.as_ref().to_owned(), + expr.expression.as_ref().to_owned(), + ); } } diff --git a/rust/perspective-viewer/src/rust/session.rs b/rust/perspective-viewer/src/rust/session.rs index c0515d4559..032b82388d 100644 --- a/rust/perspective-viewer/src/rust/session.rs +++ b/rust/perspective-viewer/src/rust/session.rs @@ -18,7 +18,7 @@ mod view; mod view_subscription; use std::cell::{Ref, RefCell}; -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use std::iter::IntoIterator; use std::ops::Deref; use std::rc::Rc; @@ -411,14 +411,7 @@ impl Session { .ok_or("`restore()` called before `load()`")? .clone(); - let arr = JsValue::from_serde_ext( - &config - .expressions - .iter() - .map(|expr| (expr.name.to_string(), expr.expression.to_string())) - .collect::>(), - )? - .unchecked_into::(); + let arr = JsValue::from_serde_ext(&config.expressions)?.unchecked_into::(); let valid_recs = table.validate_expressions(arr).await?; let expression_names = self.metadata_mut().update_expressions(&valid_recs)?; diff --git a/rust/perspective-viewer/src/rust/session/replace_expression_update.rs b/rust/perspective-viewer/src/rust/session/replace_expression_update.rs index 2359ee09cb..b33214ed8a 100644 --- a/rust/perspective-viewer/src/rust/session/replace_expression_update.rs +++ b/rust/perspective-viewer/src/rust/session/replace_expression_update.rs @@ -42,11 +42,11 @@ impl ViewConfig { let expressions = expressions .iter() - .map(|serde_expr| { - if old_expr.name == serde_expr.name { + .map(|(serde_name, serde_expr)| { + if &old_expr.name == serde_name { expression.to_owned() } else { - serde_expr + Expression::new(serde_name, serde_expr) } }) .collect::();