From 84f71f77275cd46081e6f437d1d36ee80337eaa6 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 11 Apr 2025 16:11:56 -0700 Subject: [PATCH] get rid of arcs and weaks --- rust/pspp/src/output/pivot/mod.rs | 99 ++++++++++++++-------------- rust/pspp/src/output/pivot/output.rs | 26 +++----- rust/pspp/src/output/pivot/test.rs | 4 +- 3 files changed, 61 insertions(+), 68 deletions(-) diff --git a/rust/pspp/src/output/pivot/mod.rs b/rust/pspp/src/output/pivot/mod.rs index dc8cfef2dd..a41466af12 100644 --- a/rust/pspp/src/output/pivot/mod.rs +++ b/rust/pspp/src/output/pivot/mod.rs @@ -35,7 +35,7 @@ use std::{ iter::{once, repeat, repeat_n, FusedIterator}, ops::{Index, IndexMut, Not, Range, RangeInclusive}, str::{from_utf8, FromStr, Utf8Error}, - sync::{Arc, OnceLock, Weak}, + sync::{Arc, OnceLock}, }; use binrw::Error as BinError; @@ -307,7 +307,7 @@ pub struct Dimension { /// /// The root must always be a group, although it is allowed to have no /// subcategories. - pub root: Arc, + pub root: Group, /// Ordering of leaves for presentation. /// @@ -318,6 +318,12 @@ pub struct Dimension { hide_all_labels: bool, } +pub type GroupVec<'a> = SmallVec<[&'a Group; 4]>; +pub struct Path<'a> { + groups: GroupVec<'a>, + leaf: &'a Leaf, +} + impl Dimension { pub fn is_empty(&self) -> bool { self.len() == 0 @@ -331,6 +337,10 @@ impl Dimension { self.root.nth_leaf(index) } + pub fn leaf_path(&self, index: usize) -> Option> { + self.root.leaf_path(index, SmallVec::new()) + } + pub fn builder(axis: Axis3, root: GroupBuilder) -> DimensionBuilder { DimensionBuilder::new(axis, root) } @@ -338,7 +348,6 @@ impl Dimension { #[derive(Clone, Debug)] pub struct Group { - parent: Option>, len: usize, name: Box, @@ -357,10 +366,6 @@ impl Group { GroupBuilder::new(name) } - pub fn parent(&self) -> Option> { - self.parent.as_ref().map(|parent| parent.upgrade().unwrap()) - } - pub fn nth_leaf(&self, mut index: usize) -> Option<&Leaf> { for child in &self.children { let len = child.len(); @@ -372,6 +377,18 @@ impl Group { None } + pub fn leaf_path<'a>(&'a self, mut index: usize, mut groups: GroupVec<'a>) -> Option> { + for child in &self.children { + let len = child.len(); + if index < len { + groups.push(self); + return child.leaf_path(index, groups); + } + index -= len; + } + None + } + pub fn len(&self) -> usize { self.len } @@ -389,7 +406,7 @@ pub struct DimensionBuilder { impl DimensionBuilder { pub fn new(axis: Axis3, root: GroupBuilder) -> Self { - let root = root.build(None); + let root = root.build(true); Self { axis, dimension: Dimension { @@ -449,21 +466,16 @@ impl GroupBuilder { fn len(&self) -> usize { self.children.iter().map(|category| category.len()).sum() } - fn build(self, parent: Option>) -> Arc { - Arc::new_cyclic(|weak| Group { + fn build(self, is_root: bool) -> Group { + Group { len: self.children.iter().map(|c| c.len()).sum(), name: self.name, - children: self - .children - .into_iter() - .map(|c| c.build(weak.clone())) - .collect(), + children: self.children.into_iter().map(|c| c.build()).collect(), show_label: { // By default, nested group labels are shown, but not dimension root labels. - self.show_label.unwrap_or(parent.is_some()) + self.show_label.unwrap_or(!is_root) }, - parent, - }) + } } } @@ -483,15 +495,11 @@ impl CategoryBuilder { CategoryBuilder::Leaf { .. } => 1, } } - fn build(self, parent: Weak) -> Category { + fn build(self) -> Category { match self { - Self::Group(group) => Category::Group(group.build(Some(parent))), + Self::Group(group) => Category::Group(group.build(false)), Self::Leaf { name, class } => { - let leaf = Arc::new(Leaf { - parent, - name, - class, - }); + let leaf = Arc::new(Leaf { name, class }); Category::Leaf(leaf) } } @@ -626,7 +634,6 @@ impl PivotTableBuilder { #[derive(Clone, Debug)] pub struct Leaf { - parent: Weak, name: Box, /// Default format for values in this category. @@ -636,7 +643,6 @@ pub struct Leaf { impl Leaf { pub fn new(name: Value) -> Self { Self { - parent: Weak::new(), name: Box::new(name), class: None, } @@ -647,9 +653,6 @@ impl Leaf { ..self } } - pub fn ancestors(&self) -> impl Iterator> { - std::iter::successors(self.parent(), |group| group.parent()) - } } /// Pivot result classes. @@ -670,7 +673,7 @@ pub enum Class { /// A pivot_category is a leaf (a category) or a group. #[derive(Clone, Debug)] pub enum Category { - Group(Arc), + Group(Group), Leaf(Arc), } @@ -678,7 +681,7 @@ impl Category { fn len(&self) -> usize { match self { Category::Group(group) => group.len, - Category::Leaf(leaf) => 1, + Category::Leaf(_) => 1, } } @@ -695,6 +698,22 @@ impl Category { } } + fn leaf_path<'a>(&'a self, index: usize, groups: GroupVec<'a>) -> Option> { + match self { + Category::Group(group) => group.leaf_path(index, groups), + Category::Leaf(leaf) => { + if index == 0 { + Some(Path { + groups, + leaf: &leaf, + }) + } else { + None + } + } + } + } + fn show_label(&self) -> bool { match self { Category::Group(group) => group.show_label, @@ -705,27 +724,18 @@ impl Category { trait CategoryTrait { fn name(&self) -> &Value; - fn parent(&self) -> Option>; } impl CategoryTrait for Group { fn name(&self) -> &Value { &self.name } - - fn parent(&self) -> Option> { - self.parent.as_ref().and_then(|parent| parent.upgrade()) - } } impl CategoryTrait for Leaf { fn name(&self) -> &Value { &self.name } - - fn parent(&self) -> Option> { - Some(self.parent.upgrade().unwrap()) - } } impl CategoryTrait for Category { @@ -735,13 +745,6 @@ impl CategoryTrait for Category { Category::Leaf(leaf) => leaf.name(), } } - - fn parent(&self) -> Option> { - match self { - Category::Group(group) => group.parent(), - Category::Leaf(leaf) => leaf.parent(), - } - } } /// Styling for a pivot table. diff --git a/rust/pspp/src/output/pivot/output.rs b/rust/pspp/src/output/pivot/output.rs index da60afdb7a..e66fb8ecd9 100644 --- a/rust/pspp/src/output/pivot/output.rs +++ b/rust/pspp/src/output/pivot/output.rs @@ -2,10 +2,9 @@ use std::{ops::Range, sync::Arc}; use enum_map::{enum_map, EnumMap}; use itertools::Itertools; -use smallvec::SmallVec; use crate::output::{ - pivot::{Group, HeadingRegion, LabelPosition, Leaf}, + pivot::{HeadingRegion, LabelPosition, Path}, table::{CellInner, Table}, }; @@ -324,12 +323,7 @@ pub struct OutputTables { pub footnotes: Option, } -struct HeadingColumn<'a> { - leaf: &'a Leaf, - groups: SmallVec<[Arc; 4]>, -} - -impl HeadingColumn<'_> { +impl Path<'_> { pub fn get(&self, y: usize, height: usize) -> Option<&Value> { if y + 1 == height { Some(&self.leaf.name) @@ -342,7 +336,7 @@ impl HeadingColumn<'_> { struct Heading<'a> { dimension: &'a Dimension, height: usize, - columns: Vec>, + columns: Vec>, } impl<'a> Heading<'a> { @@ -358,16 +352,12 @@ impl<'a> Heading<'a> { let mut columns = Vec::new(); let mut height = 0; for indexes in column_enumeration.iter() { - let leaf = dimension - .nth_leaf(dimension.presentation_order[indexes[dim_index]]) + let mut path = dimension + .leaf_path(dimension.presentation_order[indexes[dim_index]]) .unwrap(); - let mut groups = leaf - .ancestors() - .filter(|group| group.show_label) - .collect::>(); - groups.reverse(); - height = height.max(1 + groups.len()); - columns.push(HeadingColumn { leaf, groups }); + path.groups.retain(|group| group.show_label); + height = height.max(1 + path.groups.len()); + columns.push(path); } Some(Self { diff --git a/rust/pspp/src/output/pivot/test.rs b/rust/pspp/src/output/pivot/test.rs index acc497ddfc..3aecd0481a 100644 --- a/rust/pspp/src/output/pivot/test.rs +++ b/rust/pspp/src/output/pivot/test.rs @@ -660,13 +660,13 @@ fn empty_dimensions() { #[test] fn empty_groups() { - let mut a = GroupBuilder::new(Value::new_text("a")) + let a = GroupBuilder::new(Value::new_text("a")) .with(Value::new_text("a1")) .with(GroupBuilder::new(Value::new_text("a2"))) .with(Value::new_text("a3")); let d1 = DimensionBuilder::new(Axis3::X, a); - let mut b = GroupBuilder::new(Value::new_text("b")) + let b = GroupBuilder::new(Value::new_text("b")) .with(GroupBuilder::new(Value::new_text("b1"))) .with(Value::new_text("b2")) .with(Value::new_text("b3")); -- 2.30.2