From: Ben Pfaff Date: Wed, 7 Jan 2026 17:08:45 +0000 (-0800) Subject: more fixes X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=43c3b8cf609cc4acd8ab154a0298d95530bbdb51;p=pspp more fixes --- diff --git a/rust/pspp/src/output/pivot.rs b/rust/pspp/src/output/pivot.rs index d37fc0732f..6a033c451d 100644 --- a/rust/pspp/src/output/pivot.rs +++ b/rust/pspp/src/output/pivot.rs @@ -168,18 +168,12 @@ impl PivotTable { /// [with_title]: Self::with_title /// [with_data]: Self::with_data /// [insert]: Self::insert - pub fn new(dimensions: impl IntoIterator) -> Self { - let mut dims = Vec::new(); - let mut axes = EnumMap::::default(); - for (axis, dimension) in dimensions { - axes[axis].dimensions.push(dims.len()); - dims.push(dimension); - } + pub fn new(structure: impl Into) -> Self { + let structure = structure.into(); Self { style: PivotTableStyle::default().with_look(Settings::global().look.clone()), - layer: repeat_n(0, axes[Axis3::Z].dimensions.len()).collect(), - axes, - dimensions: dims, + layer: repeat_n(0, structure.axes[Axis3::Z].dimensions.len()).collect(), + structure, ..Self::default() } } @@ -377,7 +371,7 @@ impl PivotTable { /// Returns an iterator for all the values along `axis`. fn axis_values(&self, axis: Axis3) -> AxisIterator { AxisIterator { - indexes: repeat_n(0, self.axes[axis].dimensions.len()).collect(), + indexes: repeat_n(0, self.structure.axes[axis].dimensions.len()).collect(), lengths: self.axis_dimensions(axis).map(|d| d.len()).collect(), done: self.axis_extent(axis) == 0, } @@ -394,12 +388,12 @@ impl PivotTable { /// Returns the pivot table's dimensions. pub fn dimensions(&self) -> &[Dimension] { - &self.dimensions + &self.structure.dimensions } /// Returns the pivot table's axes. pub fn axes(&self) -> &EnumMap { - &self.axes + &self.structure.axes } /// Returns the pivot table's [Look], for modification. @@ -443,7 +437,7 @@ impl PivotTable { where C: CellIndex, { - cell_index.cell_index(self.dimensions.iter().map(|d| d.len())) + cell_index.cell_index(self.dimensions().iter().map(|d| d.len())) } /// Inserts a cell with the given `value` and `cell_index`. @@ -478,14 +472,14 @@ impl PivotTable { &self, presentation_indexes: EnumMap, ) -> SmallVec<[usize; 4]> { - let mut data_indexes = SmallVec::from_elem(0, self.dimensions.len()); + let mut data_indexes = SmallVec::from_elem(0, self.dimensions().len()); for (axis, presentation_indexes) in presentation_indexes { - for (&dim_index, &pindex) in self.axes[axis] + for (&dim_index, &pindex) in self.structure.axes[axis] .dimensions .iter() .zip(presentation_indexes.iter()) { - data_indexes[dim_index] = self.dimensions[dim_index].ptod[pindex]; + data_indexes[dim_index] = self.structure.dimensions[dim_index].ptod[pindex]; } } data_indexes @@ -507,7 +501,7 @@ impl PivotTable { /// Transposes row and columns. pub fn transpose(&mut self) { - self.axes.swap(Axis3::X, Axis3::Y); + self.structure.axes.swap(Axis3::X, Axis3::Y); } /// Returns an iterator through dimensions on the given `axis`. @@ -515,17 +509,17 @@ impl PivotTable { &self, axis: Axis3, ) -> impl DoubleEndedIterator + ExactSizeIterator { - self.axes[axis] + self.structure.axes[axis] .dimensions .iter() .copied() - .map(|index| &self.dimensions[index]) + .map(|index| &self.structure.dimensions[index]) } fn find_dimension(&self, dim_index: usize) -> Option<(Axis3, usize)> { - debug_assert!(dim_index < self.dimensions.len()); + debug_assert!(dim_index < self.structure.dimensions.len()); for axis in enum_iterator::all::() { - for (position, dimension) in self.axes[axis].dimensions.iter().copied().enumerate() { + for (position, dimension) in self.axes()[axis].dimensions.iter().copied().enumerate() { if dimension == dim_index { return Some((axis, position)); } @@ -575,8 +569,10 @@ impl PivotTable { _ => (), } - self.axes[old_axis].dimensions.remove(old_position); - self.axes[new_axis] + self.structure.axes[old_axis] + .dimensions + .remove(old_position); + self.structure.axes[new_axis] .dimensions .insert(new_position, dim_index); } @@ -1724,11 +1720,9 @@ pub struct PivotTable { /// Footnotes. pub footnotes: Footnotes, - /// Dimensions. - dimensions: Vec, - - /// Axes. - axes: EnumMap, + /// Table structure. + #[serde(flatten)] + structure: Structure, /// Data. cells: HashMap, @@ -1741,13 +1735,55 @@ impl Default for PivotTable { metadata: PivotTableMetadata::default(), layer: Vec::new(), footnotes: Footnotes::new(), - dimensions: Vec::new(), - axes: EnumMap::default(), + structure: Structure::default(), cells: HashMap::new(), } } } +/// The structure of a pivot table. +#[derive(Clone, Debug, Default, Serialize)] +pub struct Structure { + /// Dimensions. + dimensions: Vec, + + /// Axes. + axes: EnumMap, +} + +/// [Structure::new] error for a missing or duplicate dimension among the axes. +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub struct InvalidAxes; + +impl Structure { + /// Creates a new `Structure` from the arguments. + /// + /// Usually [Structure::from] is easier. + pub fn new( + dimensions: Vec, + axes: EnumMap, + ) -> Result { + // XXX validate axes + Ok(Self { dimensions, axes }) + } +} + +impl From for Structure +where + T: IntoIterator, +{ + fn from(value: T) -> Self { + let mut structure = Structure::default(); + for (axis, dimension) in value { + structure.axes[axis] + .dimensions + .push(structure.dimensions.len()); + structure.dimensions.push(dimension); + } + structure + } +} + /// A type that can calculate the index into a [PivotTable]'s data hashmap. pub trait CellIndex { /// Given the pivot table's `dimensions`, returns an index. diff --git a/rust/pspp/src/output/pivot/output.rs b/rust/pspp/src/output/pivot/output.rs index b4d1db2885..6c5040295b 100644 --- a/rust/pspp/src/output/pivot/output.rs +++ b/rust/pspp/src/output/pivot/output.rs @@ -108,7 +108,7 @@ impl PivotTable { layer_indexes: &[usize], omit_empty: bool, ) -> AxisEnumeration { - let axis = &self.axes[enum_axis]; + let axis = &self.structure.axes[enum_axis]; let extent = self.axis_extent(enum_axis); let indexes = if axis.dimensions.is_empty() { vec![0] @@ -294,10 +294,10 @@ impl PivotTable { /// Constructs a [Table] for this `PivotTable`'s layer values. Returns /// `None` if the table doesn't have layers. pub fn output_layers(&self, layer_indexes: &[usize]) -> Vec { - self.axes[Axis3::Z] + self.structure.axes[Axis3::Z] .dimensions .iter() - .map(|index| &self.dimensions[*index]) + .map(|index| &self.structure.dimensions[*index]) .zip(layer_indexes) .rev() .filter(|(dimension, _)| !dimension.is_empty()) @@ -379,11 +379,11 @@ impl PivotTable { } fn nonempty_layer_dimensions(&self) -> impl Iterator { - self.axes[Axis3::Z] + self.structure.axes[Axis3::Z] .dimensions .iter() .rev() - .map(|index| &self.dimensions[*index]) + .map(|index| &self.structure.dimensions[*index]) .filter(|d| !d.root.is_empty()) } @@ -653,14 +653,18 @@ impl<'a> Headings<'a> { let column_enumeration = pt.enumerate_axis(h.into(), layer_indexes, pt.style.look.hide_empty); - let mut headings = pt.axes[h.into()] + let mut headings = pt.structure.axes[h.into()] .dimensions .iter() .copied() .enumerate() .rev() .filter_map(|(axis_index, dim_index)| { - Heading::new(&pt.dimensions[dim_index], axis_index, &column_enumeration) + Heading::new( + &pt.structure.dimensions[dim_index], + axis_index, + &column_enumeration, + ) }) .collect::>(); diff --git a/rust/pspp/src/spv/read/light.rs b/rust/pspp/src/spv/read/light.rs index 3c0851a4fd..a3c685432c 100644 --- a/rust/pspp/src/spv/read/light.rs +++ b/rust/pspp/src/spv/read/light.rs @@ -1,7 +1,7 @@ use std::{ fmt::Debug, io::{Read, Seek}, - iter::{repeat, zip}, + iter::repeat, ops::Deref, str::FromStr, sync::Arc, @@ -23,8 +23,8 @@ use crate::{ UncheckedFormat, Width, }, output::pivot::{ - self, Axis2, Axis3, FootnoteMarkerPosition, FootnoteMarkerType, Footnotes, Group, - PivotTable, PivotTableMetadata, PivotTableStyle, PrecomputedIndex, + self, Axis, Axis2, Axis3, FootnoteMarkerPosition, FootnoteMarkerType, Footnotes, Group, + PivotTable, PivotTableMetadata, PivotTableStyle, PrecomputedIndex, Structure, look::{ self, AreaStyle, BoxBorder, Color, HeadingRegion, HorzAlign, LabelPosition, Look, RowColBorder, RowParity, Stroke, VertAlign, @@ -203,14 +203,16 @@ impl LightTable { dimension }) .collect::>(); - let axes = self - .axes - .decode(dimensions.len()) - .unwrap_or_else(|warning| { + let structure = match self.axes.decode(dimensions.len()) { + Ok(axes) => { + Structure::new(dimensions, axes).expect("Axes::decode pre-validated the structure") + } + Err(warning) => { warn(warning); - vec![Axis3::Y; dimensions.len()] - }); - let pivot_table = PivotTable::new(zip(axes, dimensions)) + Structure::from(repeat(Axis3::Y).zip(dimensions)) + } + }; + let pivot_table = PivotTable::new(structure) .with_style(PivotTableStyle { look: Arc::new(self.decode_look(encoding)), rotate_inner_column_labels: self.header.rotate_inner_column_labels, @@ -1753,7 +1755,7 @@ struct Axes { } impl Axes { - fn decode(&self, n: usize) -> Result, LightWarning> { + fn decode(&self, n: usize) -> Result, LightWarning> { let actual = self.layers.len() + self.rows.len() + self.columns.len(); if actual != n { return Err(LightWarning::WrongAxisCount { @@ -1765,24 +1767,25 @@ impl Axes { }); } - let mut axes = vec![None; n]; - fn axis_dims(axis: Axis3, dimensions: &[u32]) -> impl Iterator { repeat(axis).zip(dimensions.iter().copied().map(|x| x.try_into().unwrap())) } + let mut seen = vec![false; n]; + let mut axes: EnumMap = EnumMap::default(); for (axis, index) in axis_dims(Axis3::Z, &self.layers) .chain(axis_dims(Axis3::Y, &self.rows)) .chain(axis_dims(Axis3::X, &self.columns)) { - match axes.get_mut(index) { + match seen.get_mut(index) { None => return Err(LightWarning::InvalidDimensionIndex { index, n }), - Some(Some(_)) => return Err(LightWarning::DuplicateDimensionIndex(index)), - Some(inner) => *inner = Some(axis), + Some(true) => return Err(LightWarning::DuplicateDimensionIndex(index)), + Some(other) => *other = true, } + axes[axis].dimensions.push(index); } - Ok(axes.into_iter().flatten().collect()) + Ok(axes) } } diff --git a/rust/pspp/src/spv/read/tests.rs b/rust/pspp/src/spv/read/tests.rs index fc38a10dbc..ac27ae0d3c 100644 --- a/rust/pspp/src/spv/read/tests.rs +++ b/rust/pspp/src/spv/read/tests.rs @@ -32,6 +32,12 @@ fn light3() { test_raw_spvfile("light3", None); } +/// Further check for dimensions and axes are ordered properly. +#[test] +fn light4() { + test_raw_spvfile("light4", None); +} + #[test] fn legacy1() { test_raw_spvfile("legacy1", None); diff --git a/rust/pspp/src/spv/testdata/light2.expected b/rust/pspp/src/spv/testdata/light2.expected index 5650a27ede..adb6944f24 100644 --- a/rust/pspp/src/spv/testdata/light2.expected +++ b/rust/pspp/src/spv/testdata/light2.expected @@ -4,21 +4,21 @@ │ ├──────┬────────┬───────┤ │ │ │Normal│Marginal│Extreme│ Total│ ├──────────────────────────────┼──────┼────────┼───────┼──────┤ -│Variable A Less Count │ 10│ 7.0│ 45.5%│ 52.6%│ -│ Expected Count│ 18.2%│ 20.0%│ 6.7%│ 22│ -│ % within A │ 15.0%│ 13│ 13.3│ 34.2%│ -│ % within B │ 38.0│ 100.0%│ 63.3%│ 63.3%│ -│ % of Total │100.0%│ 35.0%│ 20│ 20.0│ +│Variable A Less Count │ 10│ 8│ 4│ 22│ +│ Expected Count│ 7.0│ 7.7│ 7.3│ 22.0│ +│ % within A │ 45.5%│ 36.4%│ 18.2%│100.0%│ +│ % within B │ 52.6%│ 38.1%│ 20.0%│ 36.7%│ +│ % of Total │ 16.7%│ 13.3%│ 6.7%│ 36.7%│ │ ╶───────────────────┼──────┼────────┼───────┼──────┤ -│ More Count │ 16.7%│ 8│ 7.7│ 36.4%│ -│ Expected Count│ 22.0│ 100.0%│ 36.7%│ 36.7%│ -│ % within A │ 61.9%│ 21.7%│ 16│ 12.7│ -│ % within B │ 19│ 19.0│ 31.7%│100.0%│ -│ % of Total │ 33.3%│ 100.0%│ 33.3%│ 60│ +│ More Count │ 9│ 13│ 16│ 38│ +│ Expected Count│ 12.0│ 13.3│ 12.7│ 38.0│ +│ % within A │ 23.7%│ 34.2%│ 42.1%│100.0%│ +│ % within B │ 47.4%│ 61.9%│ 80.0%│ 63.3%│ +│ % of Total │ 15.0%│ 21.7%│ 26.7%│ 63.3%│ ├──────────────────────────────┼──────┼────────┼───────┼──────┤ -│Total Count │ 38.1%│ 13.3%│ 4│ 7.3│ -│ Expected Count│ 9│ 12.0│ 23.7%│ 47.4%│ -│ % within A │ 42.1%│ 80.0%│ 26.7%│ 38│ -│ % within B │ 31.7%│ 21│ 21.0│ 35.0%│ -│ % of Total │ 60.0│ 100.0%│ 100.0%│100.0%│ +│Total Count │ 19│ 21│ 20│ 60│ +│ Expected Count│ 19.0│ 21.0│ 20.0│ 60.0│ +│ % within A │ 31.7%│ 35.0%│ 33.3%│100.0%│ +│ % within B │100.0%│ 100.0%│ 100.0%│100.0%│ +│ % of Total │ 31.7%│ 35.0%│ 33.3%│100.0%│ ╰──────────────────────────────┴──────┴────────┴───────┴──────╯