]> pintos-os.org Git - pspp/commitdiff
work on variable sets
authorBen Pfaff <blp@cs.stanford.edu>
Sun, 13 Sep 2026 16:31:21 +0000 (09:31 -0700)
committerBen Pfaff <blp@cs.stanford.edu>
Sun, 13 Sep 2026 16:31:21 +0000 (09:31 -0700)
rust/pspp/src/spv/read/graph.rs
rust/pspp/src/spv/read/graph/var_set.rs

index ed0411b7f995a5239236234ac841dd3a7f42d04f..b6b98a4bafd843fe935f7d624f651b82472de436 100644 (file)
@@ -66,6 +66,7 @@ use crate::{
             frames::{Component, FrameType, Frames, Position},
             patterns::Patterns,
             style::{Style, StyleCycle},
+            var_set::{Expression, Operator, VarSet, VarSetError},
         },
     },
 };
@@ -112,6 +113,9 @@ pub enum GraphWarning {
         /// Value expression.
         value: String,
     },
+
+    /// Faceting expression has multiple top-level operands.
+    MultipleFacetOperands,
 }
 
 /// An error decoding a legacy graph.
@@ -575,6 +579,7 @@ impl Visualization {
                 &series,
                 &style_cycles,
                 &styles,
+                warn,
             )?);
         }
 
@@ -590,7 +595,11 @@ impl Visualization {
         series: &BTreeMap<&str, Series>,
         style_cycles: &HashMap<&str, &StyleCycle>,
         styles: &HashMap<String, Style>,
+        warn: &mut dyn FnMut(GraphWarning),
     ) -> Result<svg::node::element::Group, GraphError> {
+        for element in &graph.elements {
+            dbg!(element.var_set(graph.faceting.as_ref(), series, warn));
+        }
         let mut graph_svg = Group::new().with_id(&graph.id);
         let mut bin_widths = BTreeMap::new();
         let mut axis_domains =
@@ -1143,6 +1152,16 @@ struct SeriesValue {
     label: Option<String>,
 }
 
+impl SeriesValue {
+    fn label(&self, formatter: &Formatter) -> String {
+        if let Some(label) = &self.label {
+            label.clone()
+        } else {
+            formatter.display(&self.value).to_string()
+        }
+    }
+}
+
 impl<T> From<T> for SeriesValue
 where
     Datum<String>: From<T>,
@@ -1206,6 +1225,16 @@ impl Series {
             formatter,
         }
     }
+
+    fn new_empty() -> Self {
+        Self::new(
+            String::from("empty"),
+            vec![],
+            false,
+            Formatter::default_for_locale(Locale::for_lang("en_US")),
+        )
+    }
+
     fn with_label(self, label: Option<String>) -> Self {
         Self {
             _label: label,
@@ -1944,6 +1973,10 @@ impl VariableReference {
             facet_level: facet_levels.get(&*variable_index).map(|v| &**v),
         });
     }
+
+    fn expression(&self) -> Expression {
+        Expression::new_variable(&self.reference)
+    }
 }
 
 #[derive(Deserialize, Debug)]
@@ -2687,6 +2720,10 @@ struct RawElement {
     style: Option<String>,
     statistics: Vec<Statistic>,
 
+    /// How to combine the `positions`, either [Operator::Cross] or
+    /// [Operator::Dot].
+    operator: Operator,
+
     positions: EnumMap<Axis3, Option<Coordinate>>,
     color: Vec<ColorAesthetic>,
     style_by: Vec<StyleBy>,
@@ -2700,6 +2737,41 @@ impl RawElement {
         self.graphic = graphic;
         self
     }
+
+    fn position_expression(&self) -> Expression {
+        let mut positions = self
+            .positions
+            .values()
+            .map(|coord| {
+                coord
+                    .as_ref()
+                    .map_or_default(|coord| Expression::new_variable(&coord.variable))
+            })
+            .collect_vec();
+
+        while positions.pop_if(|e| e.is_unity()).is_some() {}
+
+        self.operator.apply(positions)
+    }
+
+    fn var_set(
+        &self,
+        faceting: Option<&Faceting>,
+        series: &BTreeMap<&str, Series>,
+        warn: &mut dyn FnMut(GraphWarning),
+    ) -> Result<VarSet, VarSetError> {
+        let position_var_set = self.position_expression().to_var_set(series)?;
+        if let Some(faceting) = faceting {
+            let facet_var_set = faceting.expression(warn).to_var_set(series)?;
+            Ok(VarSet::for_operation(
+                faceting.method.into(),
+                position_var_set,
+                facet_var_set,
+            ))
+        } else {
+            Ok(position_var_set)
+        }
+    }
 }
 
 impl<'de> Deserialize<'de> for RawElement {
@@ -2747,6 +2819,9 @@ impl<'de> Deserialize<'de> for RawElement {
             #[serde(rename = "@clip")]
             clip: Option<Clip>,
 
+            #[serde(rename = "@dot")]
+            dot: Option<bool>,
+
             #[serde(rename = "$value")]
             statistics: Vec<Statistic>,
 
@@ -2760,6 +2835,11 @@ impl<'de> Deserialize<'de> for RawElement {
         }
 
         let raw = ElementSpec::deserialize(deserializer)?;
+        let operator = if raw.position.is_empty() && raw.dot.unwrap_or(true) {
+            Operator::Dot
+        } else {
+            Operator::Cross
+        };
         let mut positions = EnumMap::from_array([raw.x, raw.y, raw.z]);
         for (position, axis) in zip(raw.position, all()) {
             positions[axis] = Some(position);
@@ -2777,6 +2857,7 @@ impl<'de> Deserialize<'de> for RawElement {
             _extreme_style: raw.extreme_style,
             style: raw.style,
             statistics: raw.statistics,
+            operator,
             positions,
             color: raw.color,
             style_by: raw.style_by,
@@ -3064,7 +3145,7 @@ impl Part {
 struct Faceting {
     /// How to derive the domain for the facet cells.
     #[serde(default, rename = "@method")]
-    _method: FacetingMethod,
+    method: FacetingMethod,
 
     #[serde(default, rename = "@augmentEmptyCells")]
     _augment_empty_cells: bool,
@@ -3092,6 +3173,16 @@ enum FacetingMethod {
     Dot,
 }
 
+impl From<FacetingMethod> for Operator {
+    fn from(value: FacetingMethod) -> Self {
+        match value {
+            FacetingMethod::Cross => Operator::Cross,
+            FacetingMethod::Nest => Operator::Nest,
+            FacetingMethod::Dot => Operator::Dot,
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 struct Facet<'a> {
     series: &'a Series,
@@ -3118,11 +3209,32 @@ impl Faceting {
         }
         faceting
     }
+
+    fn expression(&self, warn: &mut dyn FnMut(GraphWarning)) -> Expression {
+        // Collect the children other than layers.  There should be zero or one.
+        let expression_children = self
+            .children
+            .iter()
+            .filter(|child| !child.is_layer())
+            .collect_vec();
+
+        // Warn if there's more than one.
+        if expression_children.len() > 1 {
+            warn(GraphWarning::MultipleFacetOperands);
+        }
+
+        // Convert the first one to an expression.
+        expression_children
+            .into_iter()
+            .next()
+            .map_or_default(|child| child.expression())
+    }
 }
 
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 enum FacetingChild {
+    VariableReference(VariableReference),
     Dot(Cross),
     Cross(Cross),
     Nest(Nest),
@@ -3144,8 +3256,23 @@ impl FacetingChild {
             FacetingChild::Nest(nest) => {
                 nest.decode(series, facet_levels, faceting, 0, variable_index)
             }
-            FacetingChild::Layer(_layer) => { // XXX warn
-            }
+            FacetingChild::Layer(_) => todo!(),
+            FacetingChild::VariableReference(variable_reference) => variable_reference
+                .decode_faceting(series, facet_levels, faceting, 0, variable_index),
+        }
+    }
+
+    fn is_layer(&self) -> bool {
+        matches!(self, Self::Layer(_))
+    }
+
+    fn expression(&self) -> Expression {
+        match self {
+            FacetingChild::VariableReference(variable_reference) => variable_reference.expression(),
+            FacetingChild::Dot(cross) => cross.expression(Operator::Dot),
+            FacetingChild::Cross(cross) => cross.expression(Operator::Cross),
+            FacetingChild::Nest(nest) => nest.expression(),
+            FacetingChild::Layer(_) => Expression::Unity,
         }
     }
 }
@@ -3170,6 +3297,9 @@ impl Cross {
             child.decode(series, facet_levels, faceting, index, variable_index);
         }
     }
+    fn expression(&self, operator: Operator) -> Expression {
+        operator.apply(self.children.iter().map(|child| child.expression()))
+    }
 }
 
 #[derive(Deserialize, Debug)]
@@ -3213,6 +3343,15 @@ impl CrossChild {
             }
         }
     }
+
+    fn expression(&self) -> Expression {
+        match self {
+            CrossChild::Unity => Expression::Unity,
+            CrossChild::Nest(nest) => nest.expression(),
+            CrossChild::VariableReference(variable_reference) => variable_reference.expression(),
+            CrossChild::Cross(cross) => cross.expression(Operator::Cross),
+        }
+    }
 }
 
 #[derive(Deserialize, Debug)]
@@ -3241,6 +3380,14 @@ impl Nest {
             );
         }
     }
+
+    fn expression(&self) -> Expression {
+        Operator::Nest.apply(
+            self.variable_references
+                .iter()
+                .map(VariableReference::expression),
+        )
+    }
 }
 
 #[derive(Deserialize, Debug)]
@@ -4471,11 +4618,7 @@ struct FacetColumn<'a> {
 
 impl<'a> FacetColumn<'a> {
     fn label(&self, series: &Series) -> String {
-        if let Some(label) = &self.value.label {
-            label.clone()
-        } else {
-            series.formatter.display(&self.value.value).to_string()
-        }
+        self.value.label(&series.formatter)
     }
 }
 
index 31d35141f8c8c4dcb3aa83d3158eefaf458f7394..8348885e38f357978ec10ff931a15479a2c371b3 100644 (file)
@@ -1,6 +1,6 @@
 use std::{
     borrow::Cow,
-    collections::BTreeSet,
+    collections::{BTreeMap, BTreeSet},
     iter::{Peekable, chain},
     ops::{Not as _, RangeInclusive},
     str::FromStr,
@@ -17,7 +17,7 @@ use crate::spv::read::graph::Series;
 /// - `*`, `.` (middle)
 /// - `+` (lowest)
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
-enum Operator {
+pub enum Operator {
     /// `+`
     Blend,
     /// `*`
@@ -28,8 +28,18 @@ enum Operator {
     Dot,
 }
 
-#[derive(Clone, Debug, PartialEq, Eq)]
-enum Expression {
+impl Operator {
+    pub fn apply(&self, operands: impl IntoIterator<Item = Expression>) -> Expression {
+        operands
+            .into_iter()
+            .reduce(|a, b| Expression::new_operation(*self, a, b))
+            .unwrap_or_default()
+    }
+}
+
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub enum Expression {
+    #[default]
     Unity,
     Variable(String),
     Operation {
@@ -38,21 +48,46 @@ enum Expression {
     },
 }
 
+#[derive(Debug)]
+pub enum VarSetError {
+    UnknownVariable(String),
+    InvalidNest,
+}
+
 impl Expression {
-    fn new_variable(name: impl Into<String>) -> Self {
+    pub fn new_variable(name: impl Into<String>) -> Self {
         Self::Variable(name.into())
     }
-    fn new_operation(operator: Operator, operand1: Expression, operand2: Expression) -> Self {
+    pub fn new_operation(operator: Operator, operand1: Expression, operand2: Expression) -> Self {
         Self::Operation {
             operator,
             operands: Box::new([operand1, operand2]),
         }
     }
+    pub fn is_unity(&self) -> bool {
+        matches!(self, Self::Unity)
+    }
+    pub fn to_var_set(&self, series: &BTreeMap<&str, Series>) -> Result<VarSet, VarSetError> {
+        match self {
+            Expression::Unity => Ok(VarSet::new_unity()),
+            Expression::Variable(name) => {
+                let series = series
+                    .get(name.as_str())
+                    .ok_or_else(|| VarSetError::UnknownVariable(name.clone()))?;
+                Ok(VarSet::for_variable(series))
+            }
+            Expression::Operation { operator, operands } => {
+                let a = operands[0].to_var_set(series)?;
+                let b = operands[1].to_var_set(series)?;
+                Ok(VarSet::for_operation(*operator, a, b))
+            }
+        }
+    }
 }
 
 /// Error parsing an [Expression].
 #[derive(Clone, Debug, thiserror::Error, displaydoc::Display, PartialEq, Eq)]
-enum ExpressionError {
+pub enum ExpressionParseError {
     /// Empty expression.
     EmptyString,
     /// Expression was following by garbage.
@@ -66,7 +101,7 @@ enum ExpressionError {
 }
 
 impl FromStr for Expression {
-    type Err = ExpressionError;
+    type Err = ExpressionParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         #[derive(Clone, Debug, PartialEq, Eq)]
@@ -143,10 +178,10 @@ impl FromStr for Expression {
             lexer: &mut Peekable<Tokenizer>,
             predicate: P,
             inner: I,
-        ) -> Result<Expression, ExpressionError>
+        ) -> Result<Expression, ExpressionParseError>
         where
             P: Fn(Operator) -> bool,
-            I: Fn(&mut Peekable<Tokenizer>) -> Result<Expression, ExpressionError>,
+            I: Fn(&mut Peekable<Tokenizer>) -> Result<Expression, ExpressionParseError>,
         {
             let mut retval = inner(lexer)?;
             while let Some(operator) = match_operator(lexer, &predicate) {
@@ -156,13 +191,15 @@ impl FromStr for Expression {
             Ok(retval)
         }
 
-        fn blend<'a>(lexer: &mut Peekable<Tokenizer<'a>>) -> Result<Expression, ExpressionError> {
+        fn blend<'a>(
+            lexer: &mut Peekable<Tokenizer<'a>>,
+        ) -> Result<Expression, ExpressionParseError> {
             parse_operator(lexer, |op| op == Operator::Blend, cross_dot)
         }
 
         fn cross_dot<'a>(
             lexer: &mut Peekable<Tokenizer<'a>>,
-        ) -> Result<Expression, ExpressionError> {
+        ) -> Result<Expression, ExpressionParseError> {
             parse_operator(
                 lexer,
                 |op| op == Operator::Cross || op == Operator::Dot,
@@ -170,40 +207,44 @@ impl FromStr for Expression {
             )
         }
 
-        fn nest<'a>(lexer: &mut Peekable<Tokenizer<'a>>) -> Result<Expression, ExpressionError> {
+        fn nest<'a>(
+            lexer: &mut Peekable<Tokenizer<'a>>,
+        ) -> Result<Expression, ExpressionParseError> {
             parse_operator(lexer, |op| op == Operator::Nest, primary)
         }
 
-        fn primary<'a>(lexer: &mut Peekable<Tokenizer<'a>>) -> Result<Expression, ExpressionError> {
+        fn primary<'a>(
+            lexer: &mut Peekable<Tokenizer<'a>>,
+        ) -> Result<Expression, ExpressionParseError> {
             match lexer.next() {
                 Some(Token::Unity) => Ok(Expression::Unity),
                 Some(Token::Id(id)) => Ok(Expression::Variable(id)),
                 Some(Token::LeftParen) => {
                     let retval = blend(lexer)?;
                     if lexer.next_if_eq(&Token::RightParen).is_none() {
-                        return Err(ExpressionError::ExpectedRightParen);
+                        return Err(ExpressionParseError::ExpectedRightParen);
                     }
                     Ok(retval)
                 }
-                Some(_) => Err(ExpressionError::SyntaxError),
-                None => Err(ExpressionError::UnexpectedEof),
+                Some(_) => Err(ExpressionParseError::SyntaxError),
+                None => Err(ExpressionParseError::UnexpectedEof),
             }
         }
 
         let mut lexer = Tokenizer(s).peekable();
         if lexer.peek().is_none() {
-            return Err(ExpressionError::EmptyString);
+            return Err(ExpressionParseError::EmptyString);
         }
         let expression = blend(&mut lexer)?;
         if lexer.peek().is_some() {
-            return Err(ExpressionError::TrailingGarbage);
+            return Err(ExpressionParseError::TrailingGarbage);
         }
         Ok(expression)
     }
 }
 
 #[derive(Clone, Debug)]
-enum DomainComponent {
+pub enum DomainComponent {
     Categorical(Vec<String>),
     Continuous(Option<RangeInclusive<f64>>),
     Unity,
@@ -211,10 +252,32 @@ enum DomainComponent {
 
 /// Cannot blend continuous and categorical domains.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, thiserror::Error, displaydoc::Display)]
-struct BlendError;
+pub struct BlendError;
 
 impl DomainComponent {
-    fn categories(&self) -> impl Iterator<Item = &str> {
+    pub fn for_variable(series: &Series) -> Self {
+        if series.categorical {
+            Self::Categorical(
+                series
+                    .unique_values
+                    .iter()
+                    .map(|value| value.label(&series.formatter))
+                    .collect(),
+            )
+        } else {
+            Self::Continuous(
+                series
+                    .unique_values
+                    .iter()
+                    .filter_map(|value| value.value.as_number().flatten())
+                    .minmax()
+                    .into_option()
+                    .map(|(min, max)| min..=max),
+            )
+        }
+    }
+
+    pub fn categories(&self) -> impl Iterator<Item = &str> {
         match self {
             DomainComponent::Categorical(items) => Either::Left(items.iter().map(|s| s.as_str())),
             DomainComponent::Unity => Either::Right(["0.5"].into_iter()),
@@ -226,7 +289,7 @@ impl DomainComponent {
         }
     }
 
-    fn into_categories(self) -> Vec<String> {
+    pub fn into_categories(self) -> Vec<String> {
         match self {
             DomainComponent::Categorical(items) => items,
             DomainComponent::Continuous(_) => Vec::new(),
@@ -234,7 +297,7 @@ impl DomainComponent {
         }
     }
 
-    fn range(&self) -> Option<RangeInclusive<f64>> {
+    pub fn range(&self) -> Option<RangeInclusive<f64>> {
         match self {
             Self::Categorical(items) => items.is_empty().not().then(|| 1.0..=items.len() as f64),
             Self::Continuous(range) => range.clone(),
@@ -242,7 +305,7 @@ impl DomainComponent {
         }
     }
 
-    fn split(&self, s: &str) -> Option<Self> {
+    pub fn split(&self, s: &str) -> Option<Self> {
         if let Self::Categorical(items) = self {
             Some(Self::Categorical(
                 match items.into_iter().find(|element| s == *element) {
@@ -255,7 +318,7 @@ impl DomainComponent {
         }
     }
 
-    fn blend(&self, other: &Self) -> Result<Self, BlendError> {
+    pub fn blend(&self, other: &Self) -> Result<Self, BlendError> {
         fn union(
             a: &Option<RangeInclusive<f64>>,
             b: &Option<RangeInclusive<f64>>,
@@ -293,26 +356,163 @@ impl DomainComponent {
     }
 }
 
-struct Categories<'a>(&'a DomainComponent);
+#[derive(Debug)]
+pub enum Domain {
+    Empty,
+    Component(DomainComponent),
+
+    /// Cross-product domain.
+    ///
+    /// Includes all combinations of the sub-domains.
+    Cross(Box<[Domain; 2]>),
+
+    /// Nest domain.
+    ///
+    /// Nest is a subset of the cross-product of the sub-domains which only
+    /// includes the part of the cross-product that is actually present in the data.
+    Nest {
+        domains: Box<[Domain; 2]>,
+        /// The data, for limiting the cross-product.
+        data: Vec<Series>,
+    },
 
-impl<'a> Iterator for Categories<'a> {
-    type Item = Cow<'a, str>;
+    Blend(Box<[Domain; 2]>),
+}
 
-    fn next(&mut self) -> Option<Self::Item> {
-        todo!()
+impl Domain {
+    pub fn new_unity() -> Self {
+        Self::Component(DomainComponent::Unity)
+    }
+
+    pub fn for_variable(series: &Series) -> Self {
+        Self::Component(DomainComponent::for_variable(series))
+    }
+
+    pub fn cross(a: Domain, b: Domain) -> Self {
+        Self::Cross(Box::new([a, b]))
+    }
+
+    pub fn nest(a: Domain, b: Domain, data: Vec<Series>) -> Self {
+        Self::Nest {
+            domains: Box::new([a, b]),
+            data,
+        }
+    }
+
+    /// The number of domain components in this domain.
+    pub fn len(&self) -> usize {
+        match self {
+            Domain::Empty => 0,
+            Domain::Component(_) => 1,
+            Domain::Cross(domains) | Domain::Nest { domains, .. } => {
+                domains[0].len() + domains[1].len()
+            }
+            Domain::Blend(domains) => domains[0].len(),
+        }
+    }
+
+    /// The `index`th component of this domain.
+    pub fn component(&self, index: usize) -> Option<Cow<'_, DomainComponent>> {
+        match self {
+            Domain::Empty => None,
+            Domain::Component(domain_component) => {
+                (index == 0).then_some(Cow::Borrowed(domain_component))
+            }
+            Domain::Cross(domains) | Domain::Nest { domains, data: _ } => {
+                if let Some(difference) = index.checked_sub(domains[0].len()) {
+                    domains[1].component(difference)
+                } else {
+                    domains[0].component(index)
+                }
+            }
+            Domain::Blend(domains) => {
+                let a = domains[0].component(index)?;
+                let b = domains[1].component(index)?;
+                Some(Cow::Owned(a.blend(&b).ok()?))
+            }
+        }
     }
 }
 
-struct VarSet {
+#[derive(Debug)]
+pub struct VarSet {
     data: Vec<Series>,
-    //domain: Domain,
+    domain: Domain,
 }
 
+impl VarSet {
+    pub fn len(&self) -> usize {
+        self.data
+            .first()
+            .map_or_default(|series| series.values.len())
+    }
+
+    pub fn new_unity() -> Self {
+        Self {
+            data: vec![Series::new_empty()],
+            domain: Domain::new_unity(),
+        }
+    }
+
+    pub fn for_variable(series: &Series) -> Self {
+        Self {
+            data: vec![series.clone()],
+            domain: Domain::for_variable(series),
+        }
+    }
+
+    pub fn for_operation(operator: Operator, a: Self, b: Self) -> Self {
+        match operator {
+            Operator::Blend => a.blend(b),
+            Operator::Cross => a.cross(b),
+            Operator::Nest => a.nest(b),
+            Operator::Dot => a.dot(b),
+        }
+    }
+
+    pub fn cross(self, other: Self) -> Self {
+        if self.domain.len() == 0 {
+            other
+        } else if other.domain.len() == 0 {
+            self
+        } else {
+            Self {
+                data: chain(self.data, other.data).collect(),
+                domain: Domain::cross(self.domain, other.domain),
+            }
+        }
+    }
+
+    pub fn nest(self, other: Self) -> Self {
+        if self.domain.len() == 0 {
+            other
+        } else if other.domain.len() == 0 {
+            self
+        } else {
+            let data = chain(self.data, other.data).collect_vec();
+            Self {
+                domain: Domain::nest(self.domain, other.domain, data.clone()),
+                data,
+            }
+        }
+    }
+
+    pub fn dot(self, _other: Self) -> Self {
+        todo!()
+    }
+
+    pub fn blend(self, _other: Self) -> Self {
+        todo!()
+    }
+}
+
+pub struct NestError;
+
 #[cfg(test)]
 mod tests {
     use std::str::FromStr;
 
-    use crate::spv::read::graph::var_set::{Expression, ExpressionError, Operator};
+    use crate::spv::read::graph::var_set::{Expression, ExpressionParseError, Operator};
 
     #[test]
     fn parse_expression() {
@@ -430,19 +630,25 @@ mod tests {
             );
         }
 
-        assert_eq!(Expression::from_str(""), Err(ExpressionError::EmptyString));
+        assert_eq!(
+            Expression::from_str(""),
+            Err(ExpressionParseError::EmptyString)
+        );
         assert_eq!(
             Expression::from_str("x)"),
-            Err(ExpressionError::TrailingGarbage)
+            Err(ExpressionParseError::TrailingGarbage)
         );
         assert_eq!(
             Expression::from_str("(x"),
-            Err(ExpressionError::ExpectedRightParen)
+            Err(ExpressionParseError::ExpectedRightParen)
+        );
+        assert_eq!(
+            Expression::from_str("+"),
+            Err(ExpressionParseError::SyntaxError)
         );
-        assert_eq!(Expression::from_str("+"), Err(ExpressionError::SyntaxError));
         assert_eq!(
             Expression::from_str("x+"),
-            Err(ExpressionError::UnexpectedEof)
+            Err(ExpressionParseError::UnexpectedEof)
         );
     }
 }