Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions phenoxtract/src/config/config_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@ blah: "blahblah"
),
SeriesContextConfig::new("procedure_time")
.data_context(Context::TimeOfProcedure(TimeElementType::Age)),
SeriesContextConfig::new(IdentifierConfig::Single(
"multi_column".to_string(),
))
.data_context(Context::MultiContext(
vec![Context::Hpo, Context::Disease].into(),
)),
],
}),
// Second data source: Excel
Expand Down
7 changes: 5 additions & 2 deletions phenoxtract/src/config/context.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#![allow(unused_assignments)]

use crate::types::HashableSet;
use enum_try_as_inner::EnumTryAsInner;
use serde::{Deserialize, Serialize};
use std::fmt;
Expand Down Expand Up @@ -61,7 +63,6 @@ pub enum Context {
Disease,
MultiHpoId,
Onset(TimeElementType),
HpoOrDisease,
TimeOfResolution(TimeElementType),
Severity,
ObservationStatus,
Expand Down Expand Up @@ -98,6 +99,8 @@ pub enum Context {
QuantityValue,
QuantityUnit,

MultiContext(HashableSet<Context>),

#[default]
None,
//...
Expand Down Expand Up @@ -154,7 +157,6 @@ impl Context {
| ContextKind::Disease
| ContextKind::PrimarySite
| ContextKind::Hgnc
| ContextKind::HpoOrDisease
| ContextKind::Hgvs
| ContextKind::QuantitativeMeasurement
| ContextKind::QualitativeMeasurement
Expand All @@ -173,6 +175,7 @@ impl Context {
| ContextKind::TreatmentAgent
| ContextKind::RouteOfAdministration
| ContextKind::DrugType
| ContextKind::MultiContext
| ContextKind::None => None,
})
.collect()
Expand Down
2 changes: 1 addition & 1 deletion phenoxtract/src/config/try_from_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,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!")
Expand Down
1 change: 1 addition & 0 deletions phenoxtract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ mod test_suite;

pub mod phenoxtract;
pub mod transform;
pub mod types;
pub(crate) mod utils;
mod validation;
6 changes: 6 additions & 0 deletions phenoxtract/src/test_suite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ series_contexts:
single: "procedure_time"
data_context:
time_of_procedure: age
- identifier:
single: "multi_column"
data_context:
multi_context:
- hpo
- disease
"#;
pub(crate) static EXCEL_DATASOURCE_CONFIG_FILE: &[u8] = br#"
type: "excel"
Expand Down
48 changes: 32 additions & 16 deletions phenoxtract/src/transform/strategies/hpo_disease_splitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -66,11 +66,20 @@ 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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, why not use where_data_context?

.collect();

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)
)
})
})
}

Expand All @@ -84,7 +93,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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need to filter based on where_data_context instead of data_context_kind?

This code will cause problems if there are other MultiContext columns in the CDF

.collect_owned_names();

for hpo_or_disease_col_name in hpo_or_disease_col_names {
Expand Down Expand Up @@ -118,8 +127,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 =
Expand All @@ -142,7 +151,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()?;
}

Expand Down Expand Up @@ -175,7 +187,7 @@ mod tests {
"",
];

fn to_anyvalues<'a>(items: &[&'a str]) -> Vec<AnyValue<'a>> {
fn to_any_values<'a>(items: &[&'a str]) -> Vec<AnyValue<'a>> {
items
.iter()
.map(|&s| {
Expand All @@ -188,13 +200,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();
Expand Down
171 changes: 171 additions & 0 deletions phenoxtract/src/types.rs
Original file line number Diff line number Diff line change
@@ -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<T: Hash + PartialEq + Eq + Serialize>(HashSet<T>);

impl<T: Hash + Serialize + Eq> Deref for HashableSet<T> {
type Target = HashSet<T>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T: Hash + Serialize + Eq> From<HashSet<T>> for HashableSet<T> {
fn from(value: HashSet<T>) -> Self {
HashableSet(value)
}
}

impl<T: Hash + Serialize + Eq> From<Vec<T>> for HashableSet<T> {
fn from(value: Vec<T>) -> Self {
HashableSet(value.into_iter().collect())
}
}

impl<T: Hash + Eq + Serialize> Hash for HashableSet<T> {
fn hash<H: Hasher>(&self, state: &mut H) {
let mut hashes: Vec<u64> = 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<T: Hash>(value: &T) -> u64 {
let mut hasher = DefaultHasher::new();
value.hash(&mut hasher);
hasher.finish()
}

#[test]
fn from_hashset_roundtrip() {
let hs: HashSet<i32> = [1, 2, 3].iter().cloned().collect();
let wrapped = HashableSet::from(hs.clone());
assert_eq!(*wrapped, hs);
}

#[test]
fn from_vec_deduplicates() {
let wrapped: HashableSet<i32> = 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<i32> = vec![].into();
assert!(wrapped.is_empty());
}

#[test]
fn deref_exposes_hashset_methods() {
let wrapped: HashableSet<i32> = 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<i32> = vec![1, 2, 3].into();
let b: HashableSet<i32> = 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<i32> = vec![1, 2, 3].into();
let b: HashableSet<i32> = vec![1, 2, 4].into();
assert_ne!(compute_hash(&a), compute_hash(&b));
}

#[test]
fn empty_sets_have_equal_hashes() {
let a: HashableSet<i32> = vec![].into();
let b: HashableSet<i32> = vec![].into();
assert_eq!(compute_hash(&a), compute_hash(&b));
}

#[test]
fn subset_has_different_hash() {
let a: HashableSet<i32> = vec![1, 2, 3].into();
let b: HashableSet<i32> = 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<HashableSet<i32>, &str> =
std::collections::HashMap::new();
let key: HashableSet<i32> = vec![1, 2, 3].into();
map.insert(key.clone(), "hello");
assert_eq!(map[&key], "hello");

let same_key: HashableSet<i32> = vec![3, 2, 1].into();
assert_eq!(map[&same_key], "hello");
}

#[test]
fn equal_sets_regardless_of_order() {
let a: HashableSet<i32> = vec![1, 2, 3].into();
let b: HashableSet<i32> = vec![3, 2, 1].into();
assert_eq!(a, b);
}

#[test]
fn unequal_sets_are_not_equal() {
let a: HashableSet<i32> = vec![1, 2, 3].into();
let b: HashableSet<i32> = vec![1, 2].into();
assert_ne!(a, b);
}

#[test]
fn serializes_and_deserializes() {
let original: HashableSet<i32> = vec![1, 2, 3].into();
let json = serde_json::to_string(&original).unwrap();
let restored: HashableSet<i32> = serde_json::from_str(&json).unwrap();
assert_eq!(original, restored);
}

#[test]
fn deserializes_from_json_array() {
let restored: HashableSet<i32> = 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<i32> = HashableSet::default();
assert!(s.is_empty());
}

#[test]
fn clone_is_equal_and_independent() {
let a: HashableSet<i32> = vec![1, 2, 3].into();
let b = a.clone();
assert_eq!(a, b);
assert_eq!(compute_hash(&a), compute_hash(&b));
}
}
Loading