From fbafa612db606084228943a0cf2b5b3e04f5eae4 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 10:32:42 +0200 Subject: [PATCH 1/7] Context: Add MultiContext remove HpoOrDisease HpoOrDisease is to strict --- phenoxtract/src/config/context.rs | 7 +- phenoxtract/src/types.rs | 171 ++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 phenoxtract/src/types.rs diff --git a/phenoxtract/src/config/context.rs b/phenoxtract/src/config/context.rs index e97b1fdb..321d3157 100644 --- a/phenoxtract/src/config/context.rs +++ b/phenoxtract/src/config/context.rs @@ -1,4 +1,6 @@ #![allow(unused_assignments)] + +use crate::types::HashableSet; use enum_try_as_inner::EnumTryAsInner; use serde::{Deserialize, Serialize}; use std::fmt; @@ -61,7 +63,6 @@ pub enum Context { Disease, MultiHpoId, Onset(TimeElementType), - HpoOrDisease, TimeOfResolution(TimeElementType), Severity, ObservationStatus, @@ -98,6 +99,8 @@ pub enum Context { QuantityValue, QuantityUnit, + MultiContext(HashableSet), + #[default] None, //... @@ -154,7 +157,6 @@ impl Context { | ContextKind::Disease | ContextKind::PrimarySite | ContextKind::Hgnc - | ContextKind::HpoOrDisease | ContextKind::Hgvs | ContextKind::QuantitativeMeasurement | ContextKind::QualitativeMeasurement @@ -173,6 +175,7 @@ impl Context { | ContextKind::TreatmentAgent | ContextKind::RouteOfAdministration | ContextKind::DrugType + | ContextKind::MultiContext | ContextKind::None => None, }) .collect() diff --git a/phenoxtract/src/types.rs b/phenoxtract/src/types.rs new file mode 100644 index 00000000..aeff8564 --- /dev/null +++ b/phenoxtract/src/types.rs @@ -0,0 +1,171 @@ +use serde::{Deserialize, Serialize}; +use std::collections::HashSet; +use std::hash::{Hash, Hasher}; +use std::ops::Deref; + +#[derive(Debug, Clone, PartialEq, Deserialize, Default, Serialize, Eq)] +pub struct HashableSet(HashSet); + +impl Deref for HashableSet { + type Target = HashSet; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl From> for HashableSet { + fn from(value: HashSet) -> Self { + HashableSet(value) + } +} + +impl From> for HashableSet { + fn from(value: Vec) -> Self { + HashableSet(value.into_iter().collect()) + } +} + +impl Hash for HashableSet { + fn hash(&self, state: &mut H) { + let mut hashes: Vec = self + .0 + .iter() + .map(|item| { + let mut h = std::collections::hash_map::DefaultHasher::new(); + item.hash(&mut h); + h.finish() + }) + .collect(); + hashes.sort_unstable(); + hashes.hash(state); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashSet; + use std::collections::hash_map::DefaultHasher; + use std::hash::{Hash, Hasher}; + + fn compute_hash(value: &T) -> u64 { + let mut hasher = DefaultHasher::new(); + value.hash(&mut hasher); + hasher.finish() + } + + #[test] + fn from_hashset_roundtrip() { + let hs: HashSet = [1, 2, 3].iter().cloned().collect(); + let wrapped = HashableSet::from(hs.clone()); + assert_eq!(*wrapped, hs); + } + + #[test] + fn from_vec_deduplicates() { + let wrapped: HashableSet = vec![1, 2, 2, 3, 3, 3].into(); + assert_eq!(wrapped.len(), 3); + assert!(wrapped.contains(&1)); + assert!(wrapped.contains(&2)); + assert!(wrapped.contains(&3)); + } + + #[test] + fn from_empty_vec() { + let wrapped: HashableSet = vec![].into(); + assert!(wrapped.is_empty()); + } + + #[test] + fn deref_exposes_hashset_methods() { + let wrapped: HashableSet = vec![10, 20, 30].into(); + assert!(wrapped.contains(&10)); + assert!(!wrapped.contains(&99)); + assert_eq!(wrapped.len(), 3); + } + + #[test] + fn same_elements_same_hash() { + let a: HashableSet = vec![1, 2, 3].into(); + let b: HashableSet = vec![3, 1, 2].into(); // different insertion order + assert_eq!(compute_hash(&a), compute_hash(&b)); + } + + #[test] + fn different_elements_different_hash() { + let a: HashableSet = vec![1, 2, 3].into(); + let b: HashableSet = vec![1, 2, 4].into(); + assert_ne!(compute_hash(&a), compute_hash(&b)); + } + + #[test] + fn empty_sets_have_equal_hashes() { + let a: HashableSet = vec![].into(); + let b: HashableSet = vec![].into(); + assert_eq!(compute_hash(&a), compute_hash(&b)); + } + + #[test] + fn subset_has_different_hash() { + let a: HashableSet = vec![1, 2, 3].into(); + let b: HashableSet = vec![1, 2].into(); + assert_ne!(compute_hash(&a), compute_hash(&b)); + } + + #[test] + fn hashable_set_usable_as_hashmap_key() { + let mut map: std::collections::HashMap, &str> = + std::collections::HashMap::new(); + let key: HashableSet = vec![1, 2, 3].into(); + map.insert(key.clone(), "hello"); + assert_eq!(map[&key], "hello"); + + let same_key: HashableSet = vec![3, 2, 1].into(); + assert_eq!(map[&same_key], "hello"); + } + + #[test] + fn equal_sets_regardless_of_order() { + let a: HashableSet = vec![1, 2, 3].into(); + let b: HashableSet = vec![3, 2, 1].into(); + assert_eq!(a, b); + } + + #[test] + fn unequal_sets_are_not_equal() { + let a: HashableSet = vec![1, 2, 3].into(); + let b: HashableSet = vec![1, 2].into(); + assert_ne!(a, b); + } + + #[test] + fn serializes_and_deserializes() { + let original: HashableSet = vec![1, 2, 3].into(); + let json = serde_json::to_string(&original).unwrap(); + let restored: HashableSet = serde_json::from_str(&json).unwrap(); + assert_eq!(original, restored); + } + + #[test] + fn deserializes_from_json_array() { + let restored: HashableSet = serde_json::from_str("[1,2,3]").unwrap(); + assert!(restored.contains(&1)); + assert!(restored.contains(&2)); + assert!(restored.contains(&3)); + } + + #[test] + fn default_is_empty() { + let s: HashableSet = HashableSet::default(); + assert!(s.is_empty()); + } + + #[test] + fn clone_is_equal_and_independent() { + let a: HashableSet = vec![1, 2, 3].into(); + let b = a.clone(); + assert_eq!(a, b); + assert_eq!(compute_hash(&a), compute_hash(&b)); + } +} From 2e47ce016d45f4d678c15eec160bfb0a5f081f04 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 10:33:02 +0200 Subject: [PATCH 2/7] HpoDiseaseSplitterStrategy: Adjust to use MultiContext --- phenoxtract/src/lib.rs | 1 + .../strategies/hpo_disease_splitter.rs | 47 +++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/phenoxtract/src/lib.rs b/phenoxtract/src/lib.rs index 0ef92050..92b5987e 100644 --- a/phenoxtract/src/lib.rs +++ b/phenoxtract/src/lib.rs @@ -12,5 +12,6 @@ mod test_suite; pub mod phenoxtract; pub mod transform; +pub mod types; pub(crate) mod utils; mod validation; diff --git a/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs b/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs index eff2fc1e..23ba487f 100644 --- a/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs +++ b/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs @@ -11,9 +11,9 @@ use std::any::type_name; use std::collections::HashSet; use std::sync::Arc; -/// Splits a [`Context::HpoOrDisease`] column into [`Context::Hpo`] and [`Context::Disease`] columns. +/// Splits a [`Context::MultiContext(Context::Hpo, Context::Disease)`] column into [`Context::Hpo`] and [`Context::Disease`] columns. /// -/// This strategy will find every column whose context is [`Context::HpoOrDisease`] +/// This strategy will find every column whose context is [`Context::MultiContext(Context::Hpo, Context::Disease)`] /// And split it into two separate columns: a [`Context::Hpo`] column and a [`Context::Disease`] column. /// /// Hpo is prioritised: the strategy will find all Hpo labels and IDs, and then put them into the @@ -66,11 +66,21 @@ impl HpoDiseaseSplitterStrategy { impl Strategy for HpoDiseaseSplitterStrategy { fn is_valid(&self, tables: &[&mut ContextualizedDataFrame]) -> bool { tables.iter().any(|table| { - !table - .filter_columns() - .where_data_context_kind(Filter::Is(&ContextKind::HpoOrDisease)) - .collect() - .is_empty() + let series_contexts = table + .filter_series_context() + .where_data_context_kind(Filter::Is(&ContextKind::MultiContext)) + .collect(); + + for sc in series_contexts { + if let Context::MultiContext(contexts) = sc.get_data_context() + && contexts.len() == 2 + && contexts.contains(&Context::Hpo) + && contexts.contains(&Context::Disease) + { + return true; + } + } + false }) } @@ -84,7 +94,7 @@ impl Strategy for HpoDiseaseSplitterStrategy { for table in tables.iter_mut() { let hpo_or_disease_col_names = table .filter_columns() - .where_data_context_kind(Filter::Is(&ContextKind::HpoOrDisease)) + .where_data_context_kind(Filter::Is(&ContextKind::MultiContext)) .collect_owned_names(); for hpo_or_disease_col_name in hpo_or_disease_col_names { @@ -118,8 +128,8 @@ impl Strategy for HpoDiseaseSplitterStrategy { } } - let new_hpo_col_name = format!("{hpo_or_disease_col_name}_hpo"); - let new_disease_col_name = format!("{hpo_or_disease_col_name}_disease"); + let new_hpo_col_name = format!("{}_hpo", hpo_or_disease_col.name()); + let new_disease_col_name = format!("{}_disease", hpo_or_disease_col.name()); let new_hpo_col = Column::new(new_hpo_col_name.into(), new_hpo_col_data); let new_disease_col = @@ -142,7 +152,10 @@ impl Strategy for HpoDiseaseSplitterStrategy { table .builder() - .drop_scs_alongside_cols_with_context(&Context::None, &Context::HpoOrDisease)? + .drop_scs_alongside_cols_with_context( + &Context::None, + &Context::MultiContext(vec![Context::Hpo, Context::Disease].into()), + )? .build()?; } @@ -175,7 +188,7 @@ mod tests { "", ]; - fn to_anyvalues<'a>(items: &[&'a str]) -> Vec> { + fn to_any_values<'a>(items: &[&'a str]) -> Vec> { items .iter() .map(|&s| { @@ -188,13 +201,17 @@ mod tests { .collect() } - let mut values = to_anyvalues(&phenotypes); - values.extend(to_anyvalues(&diseases)); + let mut values = to_any_values(&phenotypes); + values.extend(to_any_values(&diseases)); let disease_hpo_col = Column::new("HpoAndDisease".into(), values); cdf.builder() - .insert_col_with_context(disease_hpo_col, Context::None, Context::HpoOrDisease) + .insert_col_with_context( + disease_hpo_col, + Context::None, + Context::MultiContext(vec![Context::Hpo, Context::Disease].into()), + ) .unwrap() .build() .unwrap(); From 4f3a9c9878bcd8b54597bb1453e96add57e21ddc Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 10:33:17 +0200 Subject: [PATCH 3/7] Add Config example for MultiContext --- phenoxtract/src/config/config_loader.rs | 6 ++++++ phenoxtract/src/config/try_from_config.rs | 2 +- phenoxtract/src/test_suite/config.rs | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/phenoxtract/src/config/config_loader.rs b/phenoxtract/src/config/config_loader.rs index e895e9e6..03b1cf95 100644 --- a/phenoxtract/src/config/config_loader.rs +++ b/phenoxtract/src/config/config_loader.rs @@ -201,6 +201,12 @@ blah: "blahblah" ), SeriesContextConfig::new("procedure_time") .data_context(Context::TimeOfProcedure(TimeElementType::Age)), + SeriesContextConfig::new(IdentifierConfig::Regex( + "multi_column".to_string(), + )) + .data_context(Context::MultiContext( + vec![Context::Hpo, Context::Disease].into(), + )), ], }), // Second data source: Excel diff --git a/phenoxtract/src/config/try_from_config.rs b/phenoxtract/src/config/try_from_config.rs index 2e2be76f..d6987ce8 100644 --- a/phenoxtract/src/config/try_from_config.rs +++ b/phenoxtract/src/config/try_from_config.rs @@ -399,7 +399,7 @@ mod tests { match csv_datasource_from_config { DataSource::Csv(csv_source) => { - assert_eq!(csv_source.context.context().len(), 3); + assert_eq!(csv_source.context.context().len(), 4); } DataSource::Excel(_) => { panic!("Loaded Excel Datasource instead of Csv!") diff --git a/phenoxtract/src/test_suite/config.rs b/phenoxtract/src/test_suite/config.rs index 645d6415..57e0b483 100644 --- a/phenoxtract/src/test_suite/config.rs +++ b/phenoxtract/src/test_suite/config.rs @@ -26,6 +26,11 @@ series_contexts: - identifier: "procedure_time" data_context: time_of_procedure: age + - identifier: "multi_column" + data_context: + multi_context: + - hpo + - disease "#; pub(crate) static EXCEL_DATASOURCE_CONFIG_FILE: &[u8] = br#" type: "excel" From 03dcf90b39bf69a2c1dec8170a61c891a6f32f31 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 10:37:46 +0200 Subject: [PATCH 4/7] Docstring linting --- phenoxtract/src/transform/strategies/hpo_disease_splitter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs b/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs index 23ba487f..c1568938 100644 --- a/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs +++ b/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs @@ -45,7 +45,7 @@ use std::sync::Arc; /// /// # Errors /// -/// A [`StrategyError::MappingError`] will be thrown if any cells in the [`Context::HpoOrDisease`] column +/// A [`StrategyError::MappingError`] will be thrown if any cells in the [`Context::MultiContext`] column /// are not a label or ID in either the `hpo_bidict_lib` or the `disease_bidict_lib`. #[derive(Debug)] pub struct HpoDiseaseSplitterStrategy { From e404b1a98f690466ec5e3a2117e5f399428981eb Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 10:46:46 +0200 Subject: [PATCH 5/7] Post merge chaos --- phenoxtract/src/test_suite/config.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phenoxtract/src/test_suite/config.rs b/phenoxtract/src/test_suite/config.rs index 13fe1ca7..d2830f47 100644 --- a/phenoxtract/src/test_suite/config.rs +++ b/phenoxtract/src/test_suite/config.rs @@ -29,7 +29,8 @@ series_contexts: single: "procedure_time" data_context: time_of_procedure: age - - identifier: "multi_column" + - identifier: + single: "multi_column" data_context: multi_context: - hpo From 0c8db97d98413c5bc5350997a291056eb75f6ad0 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 10:49:35 +0200 Subject: [PATCH 6/7] Post merge chaos --- phenoxtract/src/config/config_loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phenoxtract/src/config/config_loader.rs b/phenoxtract/src/config/config_loader.rs index d7231b14..6787a7b6 100644 --- a/phenoxtract/src/config/config_loader.rs +++ b/phenoxtract/src/config/config_loader.rs @@ -201,7 +201,7 @@ blah: "blahblah" ), SeriesContextConfig::new("procedure_time") .data_context(Context::TimeOfProcedure(TimeElementType::Age)), - SeriesContextConfig::new(IdentifierConfig::Regex( + SeriesContextConfig::new(IdentifierConfig::Single( "multi_column".to_string(), )) .data_context(Context::MultiContext( From a68ad0dda51887ee0be324f3fbd359f9cf0374f3 Mon Sep 17 00:00:00 2001 From: SmartMonkey Date: Thu, 21 May 2026 11:24:13 +0200 Subject: [PATCH 7/7] nicer code --- .../strategies/hpo_disease_splitter.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs b/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs index c1568938..d3e31dbf 100644 --- a/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs +++ b/phenoxtract/src/transform/strategies/hpo_disease_splitter.rs @@ -71,16 +71,15 @@ impl Strategy for HpoDiseaseSplitterStrategy { .where_data_context_kind(Filter::Is(&ContextKind::MultiContext)) .collect(); - for sc in series_contexts { - if let Context::MultiContext(contexts) = sc.get_data_context() - && contexts.len() == 2 - && contexts.contains(&Context::Hpo) - && contexts.contains(&Context::Disease) - { - return true; - } - } - false + series_contexts.iter().any(|sc| { + matches!( + sc.get_data_context(), + Context::MultiContext(contexts) + if contexts.len() == 2 + && contexts.contains(&Context::Hpo) + && contexts.contains(&Context::Disease) + ) + }) }) }