From 80196f8a53466601d19f3f2112d927009b25912b Mon Sep 17 00:00:00 2001 From: Palash Tyagi <23239946+Magnus167@users.noreply.github.com> Date: Mon, 2 Jun 2025 20:10:39 +0100 Subject: [PATCH 1/4] Enhance element-wise and bitwise operation implementations for Frame and Frame --- src/frame/base.rs | 175 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 154 insertions(+), 21 deletions(-) diff --git a/src/frame/base.rs b/src/frame/base.rs index 49b873d..daad7ee 100644 --- a/src/frame/base.rs +++ b/src/frame/base.rs @@ -781,14 +781,13 @@ impl IndexMut<&str> for Frame { /// Panics if column labels or row indices differ between operands. macro_rules! impl_elementwise_frame_op { ($OpTrait:ident, $method:ident) => { + // &Frame $OpTrait &Frame impl<'a, 'b, T> std::ops::$OpTrait<&'b Frame> for &'a Frame where T: Clone + PartialEq + std::ops::$OpTrait, { type Output = Frame; - fn $method(self, rhs: &'b Frame) -> Frame { - // Verify matching schema if self.column_names != rhs.column_names { panic!( "Element-wise {}: column names do not match. Left: {:?}, Right: {:?}", @@ -805,21 +804,47 @@ macro_rules! impl_elementwise_frame_op { rhs.index ); } - - // Apply the matrix operation let result_matrix = (&self.matrix).$method(&rhs.matrix); - - // Determine index for the result let new_index = match self.index { RowIndex::Range(_) => None, _ => Some(self.index.clone()), }; - Frame::new(result_matrix, self.column_names.clone(), new_index) } } + // Frame $OpTrait &Frame + impl<'b, T> std::ops::$OpTrait<&'b Frame> for Frame + where + T: Clone + PartialEq + std::ops::$OpTrait, + { + type Output = Frame; + fn $method(self, rhs: &'b Frame) -> Frame { + (&self).$method(rhs) + } + } + // &Frame $OpTrait Frame + impl<'a, T> std::ops::$OpTrait> for &'a Frame + where + T: Clone + PartialEq + std::ops::$OpTrait, + { + type Output = Frame; + fn $method(self, rhs: Frame) -> Frame { + self.$method(&rhs) + } + } + // Frame $OpTrait Frame + impl std::ops::$OpTrait> for Frame + where + T: Clone + PartialEq + std::ops::$OpTrait, + { + type Output = Frame; + fn $method(self, rhs: Frame) -> Frame { + (&self).$method(&rhs) + } + } }; } + impl_elementwise_frame_op!(Add, add); impl_elementwise_frame_op!(Sub, sub); impl_elementwise_frame_op!(Mul, mul); @@ -830,11 +855,10 @@ impl_elementwise_frame_op!(Div, div); /// Panics if column labels or row indices differ between operands. macro_rules! impl_bitwise_frame_op { ($OpTrait:ident, $method:ident) => { + // &Frame $OpTrait &Frame impl<'a, 'b> std::ops::$OpTrait<&'b Frame> for &'a Frame { type Output = Frame; - fn $method(self, rhs: &'b Frame) -> Frame { - // Verify matching schema if self.column_names != rhs.column_names { panic!( "Bitwise {}: column names do not match. Left: {:?}, Right: {:?}", @@ -851,25 +875,43 @@ macro_rules! impl_bitwise_frame_op { rhs.index ); } - - // Apply the matrix operation let result_matrix = (&self.matrix).$method(&rhs.matrix); - - // Determine index for the result let new_index = match self.index { RowIndex::Range(_) => None, _ => Some(self.index.clone()), }; - Frame::new(result_matrix, self.column_names.clone(), new_index) } } + // Frame $OpTrait &Frame + impl<'b> std::ops::$OpTrait<&'b Frame> for Frame { + type Output = Frame; + fn $method(self, rhs: &'b Frame) -> Frame { + (&self).$method(rhs) + } + } + // &Frame $OpTrait Frame + impl<'a> std::ops::$OpTrait> for &'a Frame { + type Output = Frame; + fn $method(self, rhs: Frame) -> Frame { + self.$method(&rhs) + } + } + // Frame $OpTrait Frame + impl std::ops::$OpTrait> for Frame { + type Output = Frame; + fn $method(self, rhs: Frame) -> Frame { + (&self).$method(&rhs) + } + } }; } + impl_bitwise_frame_op!(BitAnd, bitand); impl_bitwise_frame_op!(BitOr, bitor); impl_bitwise_frame_op!(BitXor, bitxor); +/* ---------- Logical NOT ---------- */ /// Implements logical NOT (`!`) for `Frame`, consuming the frame. impl Not for Frame { type Output = Frame; @@ -888,12 +930,30 @@ impl Not for Frame { } } -// --- Tests --- +/// Implements logical NOT (`!`) for `&Frame`, borrowing the frame. +impl Not for &Frame { + type Output = Frame; + + fn not(self) -> Frame { + // Apply NOT to the underlying matrix + let result_matrix = !&self.matrix; + + // Determine index for the result + let new_index = match self.index { + RowIndex::Range(_) => None, + _ => Some(self.index.clone()), + }; + + Frame::new(result_matrix, self.column_names.clone(), new_index) + } +} + +// --- Tests --- #[cfg(test)] mod tests { use super::*; // Assume Matrix is available from crate::matrix or similar - use crate::matrix::Matrix; + use crate::matrix::{BoolOps, Matrix}; use chrono::NaiveDate; // HashMap needed for direct inspection in tests if required use std::collections::HashMap; @@ -1295,7 +1355,7 @@ mod tests { #[test] fn frame_row_mutate_default_index() { let mut frame = create_test_frame_f64(); // Index 0..3, A=[1,2,3], B=[4,5,6] - // Mutate using set("col_name", value) + // Mutate using set("col_name", value) frame.get_row_mut(1).set("A", 2.9); // Mutate row index 1, col A assert_eq!(frame["A"], vec![1.0, 2.9, 3.0]); // Mutate using IndexMut by physical column index @@ -1349,7 +1409,7 @@ mod tests { fn test_row_view_name_panic() { let frame = create_test_frame_f64(); let row_view = frame.get_row(0); - let _ = row_view["C"]; // Access non-existent column name + let _ = row_view["C"]; // Access non-existent column Z } #[test] #[should_panic(expected = "column index 3 out of bounds")] // Check specific message @@ -1438,7 +1498,7 @@ mod tests { assert_eq!(frame.cols(), 2); assert_eq!(frame["X"], vec![1, 2]); // X data unchanged assert_eq!(frame["B"], vec![5, 6]); // B data unchanged - // Check internal state after delete + rebuild_col_lookup + // Check internal state after delete + rebuild_col_lookup assert_eq!(frame.column_index("X"), Some(0)); // X is now physical col 0 assert_eq!(frame.column_index("B"), Some(1)); // B is now physical col 1 assert!(frame.column_index("A").is_none()); @@ -1447,10 +1507,10 @@ mod tests { // Sort Columns [X, B] -> [B, X] frame.sort_columns(); assert_eq!(frame.columns(), &["B", "X"]); // Alphabetical order of names - // Verify data remained with the correct logical column after sort + // Verify data remained with the correct logical column after sort assert_eq!(frame["B"], vec![5, 6], "Data in B after sort"); // B should still have [5, 6] assert_eq!(frame["X"], vec![1, 2], "Data in X after sort"); // X should still have [1, 2] - // Verify internal lookup map is correct after sort + // Verify internal lookup map is correct after sort assert_eq!(frame.column_index("B"), Some(0), "Index of B after sort"); // B is now physical col 0 assert_eq!(frame.column_index("X"), Some(1), "Index of X after sort"); // X is now physical col 1 assert_eq!(frame.col_lookup.len(), 2); @@ -1666,6 +1726,79 @@ mod tests { assert_eq!(frame_div["Y"], vec![10, -10]); } + #[test] + fn tests_for_frame_arithmetic_ops() { + let ops: Vec<( + &str, + fn(&Frame, &Frame) -> Frame, + fn(&Frame, &Frame) -> Frame, + )> = vec![ + ("addition", |a, b| a + b, |a, b| (&*a) + (&*b)), + ("subtraction", |a, b| a - b, |a, b| (&*a) - (&*b)), + ("multiplication", |a, b| a * b, |a, b| (&*a) * (&*b)), + ("division", |a, b| a / b, |a, b| (&*a) / (&*b)), + ]; + + for (op_name, owned_op, ref_op) in ops { + let f1 = create_test_frame_f64(); + let f2 = create_test_frame_f64_alt(); + let result_owned = owned_op(&f1, &f2); + let expected = ref_op(&f1, &f2); + + assert_eq!( + result_owned.columns(), + f1.columns(), + "Column mismatch for {}", + op_name + ); + assert_eq!( + result_owned.index(), + f1.index(), + "Index mismatch for {}", + op_name + ); + + let bool_mat = result_owned.matrix().eq_elem(expected.matrix().clone()); + assert!(bool_mat.all(), "Element-wise {} failed", op_name); + } + } + + // test not , and or on frame + #[test] + fn tests_for_frame_bool_ops() { + let ops: Vec<( + &str, + fn(&Frame, &Frame) -> Frame, + fn(&Frame, &Frame) -> Frame, + )> = vec![ + ("and", |a, b| a & b, |a, b| (&*a) & (&*b)), + ("or", |a, b| a | b, |a, b| (&*a) | (&*b)), + ("xor", |a, b| a ^ b, |a, b| (&*a) ^ (&*b)), + ]; + for (op_name, owned_op, ref_op) in ops { + let f1 = create_test_frame_bool(); + let f2 = create_test_frame_bool_alt(); + let result_owned = owned_op(&f1, &f2); + let expected = ref_op(&f1, &f2); + + assert_eq!( + result_owned.columns(), + f1.columns(), + "Column mismatch for {}", + op_name + ); + assert_eq!( + result_owned.index(), + f1.index(), + "Index mismatch for {}", + op_name + ); + + let bool_mat = result_owned.matrix().eq_elem(expected.matrix().clone()); + assert!(bool_mat.all(), "Element-wise {} failed", op_name); + } + } + #[test] fn test_frame_arithmetic_ops_date_index() { let dates = vec![d(2024, 1, 1), d(2024, 1, 2)]; From f1ab33ed96a4f8a6a50a8095bcf62ab2a3bf2db5 Mon Sep 17 00:00:00 2001 From: Palash Tyagi <23239946+Magnus167@users.noreply.github.com> Date: Sat, 7 Jun 2025 11:35:55 +0100 Subject: [PATCH 2/4] Fix comments to remove placeholder text in usage examples for BDatesList and DatesList --- src/utils/dateutils/bdates.rs | 8 ++++---- src/utils/dateutils/dates.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/utils/dateutils/bdates.rs b/src/utils/dateutils/bdates.rs index d53f6e2..5619e61 100644 --- a/src/utils/dateutils/bdates.rs +++ b/src/utils/dateutils/bdates.rs @@ -40,7 +40,7 @@ pub struct BDatesList { /// ```rust /// use chrono::NaiveDate; /// use std::error::Error; -/// use rustframe::utils::{BDatesList, DateFreq}; // Replace bdates with your actual crate/module name +/// use rustframe::utils::{BDatesList, DateFreq}; /// /// fn main() -> Result<(), Box> { /// let start_date = "2023-11-01".to_string(); // Wednesday @@ -68,7 +68,7 @@ pub struct BDatesList { /// ```rust /// use chrono::NaiveDate; /// use std::error::Error; -/// use rustframe::utils::{BDatesList, DateFreq}; // Replace bdates with your actual crate/module name +/// use rustframe::utils::{BDatesList, DateFreq}; /// /// fn main() -> Result<(), Box> { /// let start_date = "2024-02-28".to_string(); // Wednesday @@ -98,7 +98,7 @@ pub struct BDatesList { /// ```rust /// use chrono::NaiveDate; /// use std::error::Error; -/// use rustframe::utils::{BDatesList, DateFreq}; // Replace bdates with your actual crate/module name +/// use rustframe::utils::{BDatesList, DateFreq}; /// /// fn main() -> Result<(), Box> { /// let start_date = "2023-11-20".to_string(); // Mon, Week 47 @@ -294,7 +294,7 @@ impl BDatesList { /// ```rust /// use chrono::NaiveDate; /// use std::error::Error; -/// use rustframe::utils::{BDatesGenerator, DateFreq}; // Replace bdates with your actual crate/module name +/// use rustframe::utils::{BDatesGenerator, DateFreq}; /// /// fn main() -> Result<(), Box> { /// let start = NaiveDate::from_ymd_opt(2024, 4, 29).unwrap(); // Monday diff --git a/src/utils/dateutils/dates.rs b/src/utils/dateutils/dates.rs index 1bf516a..2fa6f7f 100644 --- a/src/utils/dateutils/dates.rs +++ b/src/utils/dateutils/dates.rs @@ -160,7 +160,7 @@ enum GroupKey { /// ```rust /// use chrono::NaiveDate; /// use std::error::Error; -/// # use rustframe::utils::{DatesList, DateFreq}; // Assuming the crate/module is named 'dates' +/// use rustframe::utils::{DatesList, DateFreq}; /// /// # fn main() -> Result<(), Box> { /// let start_date = "2023-11-01".to_string(); // Wednesday From 9279939c3081d619f61b9ce81209739a6fb1a0fe Mon Sep 17 00:00:00 2001 From: Palash Tyagi <23239946+Magnus167@users.noreply.github.com> Date: Sat, 7 Jun 2025 13:09:32 +0100 Subject: [PATCH 3/4] Replace BDatesList and BDateFreq with DatesList and DateFreq in benchmarks --- benches/benchmarks.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index 93cf8cd..e8954c8 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -5,7 +5,7 @@ use criterion::{criterion_group, criterion_main, Criterion}; use rustframe::{ frame::{Frame, RowIndex}, matrix::{BoolMatrix, Matrix, SeriesOps}, - utils::{BDatesList, BDateFreq}, + utils::{DateFreq, DatesList}, }; use std::time::Duration; @@ -146,7 +146,7 @@ fn matrix_operations_benchmark(c: &mut Criterion, sizes: &[usize]) { fn generate_frame(size: usize) -> Frame { let data: Vec = (0..size * size).map(|x| x as f64).collect(); let dates: Vec = - BDatesList::from_n_periods("2000-01-01".to_string(), BDateFreq::Daily, size) + DatesList::from_n_periods("2000-01-01".to_string(), DateFreq::Daily, size) .unwrap() .list() .unwrap(); From 7b33b5b58286c17fb556f758d0a5aaead0ebd211 Mon Sep 17 00:00:00 2001 From: Palash Tyagi <23239946+Magnus167@users.noreply.github.com> Date: Sat, 7 Jun 2025 13:17:06 +0100 Subject: [PATCH 4/4] Handle missing benchmark artifact by creating an HTML placeholder instead of failing the workflow --- .github/workflows/docs-and-testcov.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/docs-and-testcov.yml b/.github/workflows/docs-and-testcov.yml index c946b9b..68bd58c 100644 --- a/.github/workflows/docs-and-testcov.yml +++ b/.github/workflows/docs-and-testcov.yml @@ -112,14 +112,14 @@ jobs: if [ -z "$artifact_url" ]; then echo "No benchmark artifact found!" - exit 1 + mkdir -p benchmark-report + echo 'No Benchmarks

No benchmarks available

' > benchmark-report/index.html + exit 0 fi curl -L -H "Authorization: Bearer ${{ secrets.CUSTOM_GH_TOKEN }}" \ "$artifact_url" -o benchmark-report.zip - - # Print all files in the current directory echo "Files in the current directory:" ls -al