From 65e16208dc485c29622f322cdead7b806fe1aca3 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 12:51:22 +0200 Subject: [PATCH 1/7] Rename column strat --- phenoxtract/src/config/strategy_config.rs | 4 + phenoxtract/src/error.rs | 2 + .../src/extract/contextualized_data_frame.rs | 26 +- phenoxtract/src/transform/error.rs | 2 + .../transform/strategies/column_renaming.rs | 288 ++++++++++++++++++ phenoxtract/src/transform/strategies/mod.rs | 1 + .../transform/strategies/strategy_factory.rs | 44 +++ 7 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 phenoxtract/src/transform/strategies/column_renaming.rs diff --git a/phenoxtract/src/config/strategy_config.rs b/phenoxtract/src/config/strategy_config.rs index 11518dd7..7cf704e6 100644 --- a/phenoxtract/src/config/strategy_config.rs +++ b/phenoxtract/src/config/strategy_config.rs @@ -2,6 +2,7 @@ use crate::config::context::ContextKind; use crate::ontology::resource_references::ResourceRef; use crate::transform::strategies::mapping::DefaultMapping; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] #[serde(rename_all = "snake_case")] @@ -19,4 +20,7 @@ pub enum StrategyConfig { strict: bool, }, HpoDiseaseSplitter, + ColumnRenaming { + renaming: HashMap, + }, } diff --git a/phenoxtract/src/error.rs b/phenoxtract/src/error.rs index aa7aaac4..2f82fc7c 100644 --- a/phenoxtract/src/error.rs +++ b/phenoxtract/src/error.rs @@ -25,6 +25,8 @@ pub enum ConstructionError { LoadingAliases { path: PathBuf, err: PolarsError }, #[error("Could not load Identifier because: {reason}")] Identifier { reason: String }, + #[error("Found Duplicate Column Names: {reason}")] + DuplicateColumnNames { reason: String }, #[error("Could not find config file at '{0}'")] NoConfigFileFound(PathBuf), #[error(transparent)] diff --git a/phenoxtract/src/extract/contextualized_data_frame.rs b/phenoxtract/src/extract/contextualized_data_frame.rs index becb6874..8ef8dad7 100644 --- a/phenoxtract/src/extract/contextualized_data_frame.rs +++ b/phenoxtract/src/extract/contextualized_data_frame.rs @@ -96,6 +96,20 @@ impl ContextualizedDataFrame { .collect() } + pub fn get_sc_by_col_name(&self, col_name: &str) -> Option<&SeriesContext> { + for sc in self.series_contexts() { + let cols = self.identify_columns(sc.get_identifier()); + + for col in cols.iter() { + if col.name() == col_name { + return Some(sc); + } + } + } + + None + } + pub fn filter_series_context(&'_ self) -> SeriesContextFilter<'_> { SeriesContextFilter::new(self.context.context()) } @@ -565,6 +579,10 @@ impl<'a> ContextualizedDataFrameBuilder<'a> { } } + pub fn get_inner(&self) -> &ContextualizedDataFrame { + self.cdf + } + fn mark_dirty(mut self) -> Self { self.is_dirty = true; self @@ -607,6 +625,12 @@ impl<'a> ContextualizedDataFrameBuilder<'a> { Ok(self.mark_dirty()) } + + pub fn rename_col(self, col_name: &str, new_name: &str) -> Result { + self.cdf.data.rename(col_name, new_name.into())?; + + Ok(self.mark_dirty()) + } pub fn drop_scs_alongside_cols_with_context( mut self, header_context: &Context, @@ -743,7 +767,7 @@ impl<'a> ContextualizedDataFrameBuilder<'a> { self.mark_dirty() } - fn drop_sc(self, to_remove: &Identifier) -> Self { + pub(crate) fn drop_sc(self, to_remove: &Identifier) -> Self { self.cdf .context .context_mut() diff --git a/phenoxtract/src/transform/error.rs b/phenoxtract/src/transform/error.rs index f3c9938c..1e437155 100644 --- a/phenoxtract/src/transform/error.rs +++ b/phenoxtract/src/transform/error.rs @@ -291,6 +291,8 @@ pub enum StrategyError { found_datatype: String, allowed_datatypes: Vec, }, + #[error("Could not find column with name: {column_name}")] + ColumnNotFound { column_name: String }, } impl From for StrategyError { diff --git a/phenoxtract/src/transform/strategies/column_renaming.rs b/phenoxtract/src/transform/strategies/column_renaming.rs new file mode 100644 index 00000000..27f401ba --- /dev/null +++ b/phenoxtract/src/transform/strategies/column_renaming.rs @@ -0,0 +1,288 @@ +use crate::config::table_context::{Identifier, SeriesContext}; +use crate::extract::ContextualizedDataFrame; +use crate::transform::error::StrategyError; +use crate::transform::strategies::traits::Strategy; +use std::collections::HashMap; + +/// Renames columns in one or more [`ContextualizedDataFrame`]s according to a provided mapping. +/// +/// This strategy iterates over every `(old_name, new_name)` pair in the renaming map and applies +/// the rename to all supplied tables. The [`SeriesContext`] associated with the old column is +/// preserved in full (header context, data context, fill-missing rules, alias map, and building +/// block ID); only the [`Identifier`] is updated to reflect the new name. +/// +/// # Fields +/// +/// * `renaming` - A map from existing column names to their desired new names. +/// +/// # Example +/// +/// The table +/// +/// ```csv +/// PatientId, conditions, dob +/// P001, HP:1234567, 1990-01-01 +/// P002, Arachnodactyly, 1985-06-15 +/// ``` +/// +/// with the renaming `{ "conditions" -> "phenotypes", "dob" -> "date_of_birth" }` becomes +/// +/// ```csv +/// PatientId, phenotypes, date_of_birth +/// P001, HP:1234567, 1990-01-01 +/// P002, Arachnodactyly, 1985-06-15 +/// ``` +/// +/// # Errors +/// +/// A [`StrategyError::ColumnNotFound`] will be returned if any key in the renaming map does not +/// correspond to an existing column in one of the supplied tables. +#[derive(Debug)] +pub struct ColumnRenamingStrategy { + renaming: HashMap, +} + +impl ColumnRenamingStrategy { + pub fn new(renaming: HashMap) -> Self { + ColumnRenamingStrategy { renaming } + } +} + +impl Strategy for ColumnRenamingStrategy { + fn is_valid(&self, _: &[&mut ContextualizedDataFrame]) -> bool { + true + } + + fn internal_transform( + &self, + tables: &mut [&mut ContextualizedDataFrame], + ) -> Result<(), StrategyError> { + for (old, new) in self.renaming.iter() { + for table in tables.iter_mut() { + let (new_sc, old_id) = match table.get_sc_by_col_name(old) { + Some(old_sc) => Ok(( + SeriesContext::new( + Identifier::Single(new.to_string()), + old_sc.get_header_context().clone(), + old_sc.get_data_context().clone(), + old_sc.get_fill_missing().cloned(), + old_sc.get_alias_map().cloned(), + old_sc.get_building_block_id().map(|id| id.to_string()), + ), + old_sc.get_identifier().clone(), + )), + None => Err(StrategyError::ColumnNotFound { + column_name: old.to_string(), + }), + }?; + + let mut builder = table.builder().rename_col(old, new)?.insert_sc(new_sc)?; + + if builder.get_inner().get_dangling_scs().contains(&old_id) { + builder = builder.drop_sc(&old_id); + } + + builder.build()?; + } + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::traits::SeriesContextBuilding; + use crate::test_suite::cdf_generation::generate_minimal_cdf; + use crate::transform::error::StrategyError; + use crate::transform::strategies::traits::Strategy; + use polars::prelude::Column; + use rstest::rstest; + use std::collections::HashMap; + + #[rstest] + fn test_rename_single_column() { + let mut cdf = generate_minimal_cdf(1, 3); + + // Discover whatever column name generate_minimal_cdf gave us + let original_name = cdf.data().get_column_names()[0].to_string(); + let new_name = format!("{original_name}_renamed"); + + let strategy = + ColumnRenamingStrategy::new(HashMap::from([(original_name.clone(), new_name.clone())])); + + strategy.transform(&mut [&mut cdf]).unwrap(); + + let result_names: Vec<&str> = cdf + .data() + .get_column_names() + .iter() + .map(|col_name| col_name.as_str()) + .collect(); + + assert!( + result_names.contains(&new_name.as_str()), + "Expected renamed column '{new_name}' to exist" + ); + assert!( + !result_names.contains(&original_name.as_str()), + "Old column '{original_name}' should no longer exist" + ); + } + + #[rstest] + fn test_rename_multiple_columns() { + let mut cdf = generate_minimal_cdf(1, 1); + + let mut cdf = cdf + .builder() + .insert_scs_alongside_cols(&[ + ( + SeriesContext::from_identifier("Some_col_1"), + vec![Column::new("Some_col_1".into(), vec!["Some_val"])], + ), + ( + SeriesContext::from_identifier("Some_col_2"), + vec![Column::new("Some_col_2".into(), vec!["Some_val"])], + ), + ]) + .unwrap() + .build() + .unwrap(); + + let col_names: Vec = cdf + .data() + .get_column_names() + .into_iter() + .map(|s| s.to_string()) + .collect(); + + let (old_a, old_b) = (col_names[1].clone(), col_names[2].clone()); + let (new_a, new_b) = ("alpha".to_string(), "beta".to_string()); + + let strategy = ColumnRenamingStrategy::new(HashMap::from([ + (old_a.clone(), new_a.clone()), + (old_b.clone(), new_b.clone()), + ])); + + strategy.transform(&mut [&mut cdf]).unwrap(); + + let result_names: Vec<&str> = cdf + .data() + .get_column_names() + .iter() + .map(|col_name| col_name.as_str()) + .collect(); + + assert!(result_names.contains(&"alpha"), "Expected column 'alpha'"); + assert!(result_names.contains(&"beta"), "Expected column 'beta'"); + assert!( + !result_names.contains(&old_a.as_str()), + "Old column '{old_a}' should be gone" + ); + assert!( + !result_names.contains(&old_b.as_str()), + "Old column '{old_b}' should be gone" + ); + } + + #[rstest] + fn test_rename_preserves_series_context() { + let mut cdf = generate_minimal_cdf(1, 3); + + let original_name = cdf.data().get_column_names()[0].to_string(); + let new_name = format!("{original_name}_renamed"); + + let original_sc = cdf.get_sc_by_col_name(&original_name).unwrap().clone(); + let expected_data_ctx = original_sc.get_data_context().clone(); + let expected_header_ctx = original_sc.get_header_context().clone(); + + let strategy = + ColumnRenamingStrategy::new(HashMap::from([(original_name.clone(), new_name.clone())])); + + strategy.transform(&mut [&mut cdf]).unwrap(); + + let renamed_sc = cdf + .get_sc_by_col_name(&new_name) + .expect("SeriesContext for renamed column should exist"); + + assert_eq!( + renamed_sc.get_data_context(), + &expected_data_ctx, + "Data context should be preserved after rename" + ); + assert_eq!( + renamed_sc.get_header_context(), + &expected_header_ctx, + "Header context should be preserved after rename" + ); + } + + #[rstest] + fn test_rename_missing_column_returns_error() { + let mut cdf = generate_minimal_cdf(1, 3); + + let strategy = ColumnRenamingStrategy::new(HashMap::from([( + "this_column_does_not_exist".to_string(), + "irrelevant".to_string(), + )])); + + let result = strategy.transform(&mut [&mut cdf]); + + assert!(matches!( + result, + Err(StrategyError::ColumnNotFound { column_name }) + if column_name == "this_column_does_not_exist" + )); + } + + #[rstest] + fn test_empty_renaming_map_is_noop() { + let mut cdf = generate_minimal_cdf(1, 3); + + let before: Vec = cdf + .data() + .get_column_names() + .into_iter() + .map(|s| s.to_string()) + .collect(); + + let strategy = ColumnRenamingStrategy::new(HashMap::new()); + strategy.transform(&mut [&mut cdf]).unwrap(); + + let after: Vec = cdf + .data() + .get_column_names() + .into_iter() + .map(|s| s.to_string()) + .collect(); + + assert_eq!( + before, after, + "Column names should be unchanged for empty map" + ); + } + + #[rstest] + fn test_rename_duplicate_name() { + let mut cdf = generate_minimal_cdf(1, 3); + + let original_name = cdf.data().get_column_names()[0].to_string(); + let new_name = "subject_id"; + + let strategy = ColumnRenamingStrategy::new(HashMap::from([( + original_name.clone(), + new_name.to_string(), + )])); + + match strategy.transform(&mut [&mut cdf]) { + Ok(_) => { + panic!("Should have failed, because of duplicate column names") + } + Err(err) => { + matches!(err, StrategyError::ValidationError(_)) + } + }; + } +} diff --git a/phenoxtract/src/transform/strategies/mod.rs b/phenoxtract/src/transform/strategies/mod.rs index 1db75737..3477aa2e 100644 --- a/phenoxtract/src/transform/strategies/mod.rs +++ b/phenoxtract/src/transform/strategies/mod.rs @@ -12,6 +12,7 @@ pub use date_to_age::DateToAgeStrategy; pub mod multi_hpo_col_expansion; pub use multi_hpo_col_expansion::MultiHPOColExpansionStrategy; +pub mod column_renaming; pub mod hpo_disease_splitter; pub mod strategy_factory; pub mod traits; diff --git a/phenoxtract/src/transform/strategies/strategy_factory.rs b/phenoxtract/src/transform/strategies/strategy_factory.rs index 5e87d010..8086243a 100644 --- a/phenoxtract/src/transform/strategies/strategy_factory.rs +++ b/phenoxtract/src/transform/strategies/strategy_factory.rs @@ -2,6 +2,7 @@ use crate::config::strategy_config::StrategyConfig; use crate::error::ConstructionError; use crate::ontology::CachedOntologyFactory; use crate::transform::strategies::age_to_iso8601::AgeToIso8601Strategy; +use crate::transform::strategies::column_renaming::ColumnRenamingStrategy; use crate::transform::strategies::hpo_disease_splitter::HpoDiseaseSplitterStrategy; use crate::transform::strategies::mapping::DefaultMapping; use crate::transform::strategies::traits::Strategy; @@ -11,6 +12,7 @@ use crate::transform::strategies::{ }; use crate::transform::transform_context::TransformContext; use ontology_registry::traits::OntologyRegistration; +use std::collections::HashSet; pub struct StrategyFactory { ontology_factory: CachedOntologyFactory, @@ -69,6 +71,17 @@ impl StrategyFactory { self.ctx.hpo_bidict_lib().clone(), self.ctx.disease_bidict_lib().clone(), ))), + StrategyConfig::ColumnRenaming { renaming } => { + let names: HashSet<&String> = renaming.values().collect(); + if names.len() < renaming.len() { + return Err(ConstructionError::DuplicateColumnNames { + reason: "Configured ColumnRenamingStrategy does feature duplicates." + .to_string(), + }); + } + + Ok(Box::new(ColumnRenamingStrategy::new(renaming.clone()))) + } } } @@ -87,6 +100,7 @@ mod tests { use crate::test_suite::resource_references::MONDO_REF; use crate::transform::strategies::mapping::DefaultMapping; use rstest::rstest; + use std::collections::HashMap; fn create_test_factory() -> StrategyFactory { StrategyFactory { @@ -222,4 +236,34 @@ mod tests { let _: &dyn Strategy = strategy.as_ref(); } + + #[rstest] + fn test_column_renaming_strategy_ok() { + let mut factory = create_test_factory(); + let config = StrategyConfig::ColumnRenaming { + renaming: HashMap::from_iter([ + ("1".to_string(), "2".to_string()), + ("3".to_string(), "4".to_string()), + ]), + }; + + let strategy_result = factory.try_from_config(&config); + + assert!(strategy_result.is_ok()); + } + + #[rstest] + fn test_column_renaming_strategy_err() { + let mut factory = create_test_factory(); + let config = StrategyConfig::ColumnRenaming { + renaming: HashMap::from_iter([ + ("1".to_string(), "2".to_string()), + ("3".to_string(), "2".to_string()), + ]), + }; + + let strategy_result = factory.try_from_config(&config); + + assert!(strategy_result.is_err()); + } } From 21e72973249bd301021ebdf03d1d2a86a3dca74e Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 12:53:09 +0200 Subject: [PATCH 2/7] scoping --- phenoxtract/src/extract/contextualized_data_frame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phenoxtract/src/extract/contextualized_data_frame.rs b/phenoxtract/src/extract/contextualized_data_frame.rs index 8ef8dad7..22d64623 100644 --- a/phenoxtract/src/extract/contextualized_data_frame.rs +++ b/phenoxtract/src/extract/contextualized_data_frame.rs @@ -767,7 +767,7 @@ impl<'a> ContextualizedDataFrameBuilder<'a> { self.mark_dirty() } - pub(crate) fn drop_sc(self, to_remove: &Identifier) -> Self { + pub fn drop_sc(self, to_remove: &Identifier) -> Self { self.cdf .context .context_mut() From a69aa2490c187075ba98a21da1c8764b99bcea58 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 12:55:00 +0200 Subject: [PATCH 3/7] is_valid check --- phenoxtract/src/transform/strategies/column_renaming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phenoxtract/src/transform/strategies/column_renaming.rs b/phenoxtract/src/transform/strategies/column_renaming.rs index 27f401ba..917c070c 100644 --- a/phenoxtract/src/transform/strategies/column_renaming.rs +++ b/phenoxtract/src/transform/strategies/column_renaming.rs @@ -50,7 +50,7 @@ impl ColumnRenamingStrategy { impl Strategy for ColumnRenamingStrategy { fn is_valid(&self, _: &[&mut ContextualizedDataFrame]) -> bool { - true + !self.renaming.is_empty() } fn internal_transform( From 7aaf1208497e089da3c6d72030cef323401cbe65 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 13:00:19 +0200 Subject: [PATCH 4/7] is_valid check --- .../src/transform/strategies/column_renaming.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/phenoxtract/src/transform/strategies/column_renaming.rs b/phenoxtract/src/transform/strategies/column_renaming.rs index 917c070c..175aa190 100644 --- a/phenoxtract/src/transform/strategies/column_renaming.rs +++ b/phenoxtract/src/transform/strategies/column_renaming.rs @@ -59,8 +59,8 @@ impl Strategy for ColumnRenamingStrategy { ) -> Result<(), StrategyError> { for (old, new) in self.renaming.iter() { for table in tables.iter_mut() { - let (new_sc, old_id) = match table.get_sc_by_col_name(old) { - Some(old_sc) => Ok(( + let (new_sc, old_id) = if let Some(old_sc) = table.get_sc_by_col_name(old) { + ( SeriesContext::new( Identifier::Single(new.to_string()), old_sc.get_header_context().clone(), @@ -70,11 +70,10 @@ impl Strategy for ColumnRenamingStrategy { old_sc.get_building_block_id().map(|id| id.to_string()), ), old_sc.get_identifier().clone(), - )), - None => Err(StrategyError::ColumnNotFound { - column_name: old.to_string(), - }), - }?; + ) + } else { + continue; + }; let mut builder = table.builder().rename_col(old, new)?.insert_sc(new_sc)?; From 1da646d9ace5be2018b848b0cecf201f87dddada Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 13:00:40 +0200 Subject: [PATCH 5/7] Remove error type --- phenoxtract/src/transform/error.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/phenoxtract/src/transform/error.rs b/phenoxtract/src/transform/error.rs index 1e437155..f3c9938c 100644 --- a/phenoxtract/src/transform/error.rs +++ b/phenoxtract/src/transform/error.rs @@ -291,8 +291,6 @@ pub enum StrategyError { found_datatype: String, allowed_datatypes: Vec, }, - #[error("Could not find column with name: {column_name}")] - ColumnNotFound { column_name: String }, } impl From for StrategyError { From 857c9a7b1b56e31a1cf92f2f061441d76e8c4ff9 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 13:01:31 +0200 Subject: [PATCH 6/7] Fix tests --- .../transform/strategies/column_renaming.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/phenoxtract/src/transform/strategies/column_renaming.rs b/phenoxtract/src/transform/strategies/column_renaming.rs index 175aa190..dd953165 100644 --- a/phenoxtract/src/transform/strategies/column_renaming.rs +++ b/phenoxtract/src/transform/strategies/column_renaming.rs @@ -218,24 +218,6 @@ mod tests { ); } - #[rstest] - fn test_rename_missing_column_returns_error() { - let mut cdf = generate_minimal_cdf(1, 3); - - let strategy = ColumnRenamingStrategy::new(HashMap::from([( - "this_column_does_not_exist".to_string(), - "irrelevant".to_string(), - )])); - - let result = strategy.transform(&mut [&mut cdf]); - - assert!(matches!( - result, - Err(StrategyError::ColumnNotFound { column_name }) - if column_name == "this_column_does_not_exist" - )); - } - #[rstest] fn test_empty_renaming_map_is_noop() { let mut cdf = generate_minimal_cdf(1, 3); From 890942c6df56f76d255a2b8fc900872b11fff8e8 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 13:02:13 +0200 Subject: [PATCH 7/7] Doc tests --- phenoxtract/src/transform/strategies/column_renaming.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phenoxtract/src/transform/strategies/column_renaming.rs b/phenoxtract/src/transform/strategies/column_renaming.rs index dd953165..59ee805c 100644 --- a/phenoxtract/src/transform/strategies/column_renaming.rs +++ b/phenoxtract/src/transform/strategies/column_renaming.rs @@ -32,11 +32,6 @@ use std::collections::HashMap; /// P001, HP:1234567, 1990-01-01 /// P002, Arachnodactyly, 1985-06-15 /// ``` -/// -/// # Errors -/// -/// A [`StrategyError::ColumnNotFound`] will be returned if any key in the renaming map does not -/// correspond to an existing column in one of the supplied tables. #[derive(Debug)] pub struct ColumnRenamingStrategy { renaming: HashMap,