Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Serialize Expressions to Objects #2436

Merged
merged 1 commit into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions packages/perspective-jupyterlab/test/jupyter/widget.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ describe_jupyter(
"date",
"datetime",
],
expressions: [],
expressions: {},
filter: [],
group_by: [],
plugin: "Datagrid",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -317,7 +317,7 @@ w.theme = "Pro Dark"`
"date",
"datetime",
],
expressions: [],
expressions: {},
filter: [],
group_by: [],
plugin: "Datagrid",
Expand Down
132 changes: 53 additions & 79 deletions rust/perspective-viewer/src/rust/config/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

What makes this type unique that it requires the qualifier Deserde in the name (as opposed to the other components of ViewerConfig)? There is no other Expressions type, so the fact that it only supports deserialization doesn't disambiguate it.

Array(Vec<String>),
Map(HashMap<String, String>),
}

#[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<String, String>);

impl std::ops::Deref for Expressions {
type Target = HashMap<String, String>;

impl Default for Expressions {
fn default() -> Self {
Self::Map(HashMap::default())
fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<'a> FromIterator<Expression<'a>> for Expressions {
fn from_iter<T: IntoIterator<Item = Expression<'a>>>(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<String, String> {
expressions.iter().map(|s| {
impl From<ExpressionsDeserde> 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::<HashMap<_, _>>()
}

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<Expression<'a>> for Expressions {
fn from_iter<T: IntoIterator<Item = Expression<'a>>>(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<dyn Iterator<Item = Expression<'_>> + '_> {
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(),
);
}
}
11 changes: 2 additions & 9 deletions rust/perspective-viewer/src/rust/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<HashMap<_, _>>(),
)?
.unchecked_into::<js_sys::Object>();
let arr = JsValue::from_serde_ext(&config.expressions)?.unchecked_into::<js_sys::Object>();

let valid_recs = table.validate_expressions(arr).await?;
let expression_names = self.metadata_mut().update_expressions(&valid_recs)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Expressions>();
Expand Down