Skip to content

Commit fd09915

Browse files
shepmastershepmaster
committed
Make DicomValue a homogeneous collection instead of heterogeneous
The previous type allowed for a collection of mixed integers / strings / floats, etc. Since the DICOM format doesn't allow for that, we can change the type to enforce a single inner type. We also create our own `Value` enum instead of using `rmpv::Value`. This allows us to ignore non-UTF8 strings and integers that cannot be represented as i64.
1 parent a27c17a commit fd09915

File tree

3 files changed

+114
-107
lines changed

3 files changed

+114
-107
lines changed

‎src/dicom_json.rs

+36-6
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,48 @@ pub struct Alphabetic {
1414
#[derive(Serialize, Deserialize, Debug, PartialEq)]
1515
#[serde(untagged)]
1616
pub enum DicomValue {
17-
Integer(i64),
18-
Float(f64),
19-
String(String),
20-
Alphabetic(Alphabetic),
21-
SeqField(DicomJsonData),
17+
Integer(Vec<i64>),
18+
Float(Vec<f64>),
19+
String(Vec<String>),
20+
Alphabetic(Vec<Alphabetic>),
21+
SeqField(Vec<DicomJsonData>),
22+
}
23+
24+
impl DicomValue {
25+
pub fn is_empty(&self) -> bool {
26+
match self {
27+
DicomValue::Integer(v) => v.is_empty(),
28+
DicomValue::Float(v) => v.is_empty(),
29+
DicomValue::String(v) => v.is_empty(),
30+
DicomValue::Alphabetic(v) => v.is_empty(),
31+
DicomValue::SeqField(v) => v.is_empty(),
32+
}
33+
}
34+
35+
#[cfg(test)]
36+
pub fn to_string_ref(&self) -> Option<&[String]> {
37+
if let DicomValue::String(s) = self {
38+
Some(s)
39+
} else {
40+
None
41+
}
42+
}
43+
44+
#[cfg(test)]
45+
pub fn to_alphabetic_ref(&self) -> Option<&[Alphabetic]> {
46+
if let DicomValue::Alphabetic(s) = self {
47+
Some(s)
48+
} else {
49+
None
50+
}
51+
}
2252
}
2353

2454
#[derive(Serialize, Deserialize, Debug, PartialEq)]
2555
pub struct DicomField {
2656
#[serde(rename = "Value")]
2757
#[serde(skip_serializing_if = "Option::is_none")]
28-
pub value: Option<Vec<DicomValue>>,
58+
pub value: Option<DicomValue>,
2959
#[serde(with = "vr_serialization")]
3060
pub vr: VR,
3161
#[serde(rename = "InlineBinary")]

‎src/dimble_to_ir.rs

+28-31
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::dicom_json::*;
22
use crate::ir_to_dimble::{HeaderField, HeaderFieldMap};
33
use memmap2::MmapOptions;
4-
use rmpv::{decode, Integer, Value};
4+
use serde::Deserialize;
55
use std::fs;
66

77
fn headerfield_and_bytes_to_dicom_fields(
@@ -18,8 +18,10 @@ fn headerfield_and_bytes_to_dicom_fields(
1818
HeaderField::SQ(sqs) => {
1919
let seq_fields = sqs
2020
.iter()
21-
.map(|sq| DicomValue::SeqField(headers_to_data(sq, dimble_buffer)))
22-
.collect::<Vec<_>>();
21+
.map(|sq| headers_to_data(sq, dimble_buffer))
22+
.collect();
23+
24+
let seq_fields = DicomValue::SeqField(seq_fields);
2325

2426
DicomField {
2527
value: Some(seq_fields),
@@ -50,31 +52,36 @@ fn headerfield_and_bytes_to_dicom_fields(
5052
}
5153
b"PN" => {
5254
let name = rmp_serde::decode::from_slice(field_bytes).unwrap();
53-
let a = DicomValue::Alphabetic(Alphabetic { alphabetic: name });
55+
let a = DicomValue::Alphabetic(vec![Alphabetic { alphabetic: name }]);
5456
DicomField {
55-
value: Some(vec![a]),
57+
value: Some(a),
5658
vr: *vr,
5759
inline_binary: None,
5860
}
5961
}
6062
_ => {
61-
let mut cursor = field_bytes;
62-
let v = decode::read_value(&mut cursor).unwrap();
63-
let value: Vec<_> = match v {
64-
Value::String(s) => vec![DicomValue::String(s.into_str().unwrap())],
65-
Value::Integer(i) => vec![integer_to_dicom_value(&i)],
66-
Value::F64(f) => vec![DicomValue::Float(f)],
67-
Value::Array(a) => a
68-
.into_iter()
69-
.map(|v| match v {
70-
Value::String(s) => DicomValue::String(s.into_str().unwrap()),
71-
Value::Integer(i) => integer_to_dicom_value(&i),
72-
Value::F64(f) => DicomValue::Float(f),
73-
_ => panic!("unexpected value type: {v:?}"),
74-
})
75-
.collect(),
76-
_ => panic!("unexpected value type: {v:?}"),
63+
#[derive(Debug, Deserialize)]
64+
#[serde(untagged)]
65+
enum MyValue {
66+
String(String),
67+
Strings(Vec<String>),
68+
Integer(i64), // We only support values that can be represented as i64
69+
Integers(Vec<i64>),
70+
F64(f64),
71+
F64s(Vec<f64>),
72+
}
73+
74+
let v = rmp_serde::decode::from_slice(field_bytes).unwrap();
75+
76+
let value = match v {
77+
MyValue::String(s) => DicomValue::String(vec![s]),
78+
MyValue::Strings(s) => DicomValue::String(s),
79+
MyValue::Integer(i) => DicomValue::Integer(vec![i]),
80+
MyValue::Integers(i) => DicomValue::Integer(i),
81+
MyValue::F64(f) => DicomValue::Float(vec![f]),
82+
MyValue::F64s(f) => DicomValue::Float(f),
7783
};
84+
7885
DicomField {
7986
value: Some(value),
8087
vr: *vr,
@@ -96,16 +103,6 @@ fn headers_to_data(sq: &HeaderFieldMap, dimble_buffer: &[u8]) -> DicomJsonData {
96103
.collect()
97104
}
98105

99-
fn integer_to_dicom_value(i: &Integer) -> DicomValue {
100-
if let Some(v) = i.as_i64() {
101-
DicomValue::Integer(v)
102-
} else if let Some(v) = i.as_u64() {
103-
DicomValue::Integer(v as i64)
104-
} else {
105-
panic!("Could not represent the integer as i64 or u64")
106-
}
107-
}
108-
109106
pub fn dimble_to_dicom_json(dimble_path: &str, json_path: &str) {
110107
let dimble_file = fs::File::open(dimble_path).unwrap();
111108
let dimble_buffer = unsafe { MmapOptions::new().map(&dimble_file).expect("mmap failed") };

‎src/ir_to_dimble.rs

+50-70
Original file line numberDiff line numberDiff line change
@@ -31,49 +31,28 @@ fn get_file_bytes(safetensors_path: &str) -> Vec<u8> {
3131
fs::read(safetensors_path).unwrap()
3232
}
3333

34-
fn dicom_values_to_vec(tag: &str, dicom_values: &[DicomValue]) -> Option<Vec<u8>> {
34+
fn dicom_values_to_vec(tag: &str, dicom_values: &DicomValue) -> Option<Vec<u8>> {
3535
let field_bytes = match dicom_values {
36-
[DicomValue::String(s)] => to_vec(&s),
37-
[DicomValue::Integer(u)] => to_vec(&u),
38-
[DicomValue::Float(u)] => to_vec(&u),
39-
[DicomValue::Alphabetic(u)] => to_vec(&u.alphabetic),
40-
many => match many
41-
.first()
42-
.expect("This should definitely have a first element")
43-
{
44-
DicomValue::String(_) => to_vec(
45-
&many
46-
.iter()
47-
.map(|v| match v {
48-
DicomValue::String(s) => s.to_owned(),
49-
_ => panic!("{tag} expected only strings"),
50-
})
51-
.collect::<Vec<String>>(),
52-
),
53-
DicomValue::Integer(_) => to_vec(
54-
&many
55-
.iter()
56-
.map(|v| match v {
57-
DicomValue::Integer(i) => *i,
58-
_ => panic!("{tag} expected only ints"),
59-
})
60-
.collect::<Vec<i64>>(),
61-
),
62-
DicomValue::Float(_) => to_vec(
63-
&many
64-
.iter()
65-
.map(|v| match v {
66-
DicomValue::Float(f) => *f,
67-
_ => panic!("{tag} expected only floats"),
68-
})
69-
.collect::<Vec<f64>>(),
70-
),
71-
DicomValue::SeqField(_) => {
72-
// TODO: handle sequences of sequences properly
73-
return None;
74-
}
75-
other => panic!("{tag} unexpected value type {:?}", other),
36+
DicomValue::String(v) => match &**v {
37+
[s] => to_vec(&s),
38+
o => to_vec(o),
39+
},
40+
DicomValue::Integer(v) => match &**v {
41+
[u] => to_vec(&u),
42+
o => to_vec(o),
7643
},
44+
DicomValue::Float(v) => match &**v {
45+
[u] => to_vec(&u),
46+
o => to_vec(o),
47+
},
48+
DicomValue::Alphabetic(v) => match &**v {
49+
[u] => to_vec(&u.alphabetic),
50+
_ => panic!("{tag} can't have more than one Alphabetic"),
51+
},
52+
DicomValue::SeqField(_) => {
53+
// TODO: handle sequences of sequences properly
54+
return None;
55+
}
7756
};
7857
let field_bytes = field_bytes.unwrap();
7958
Some(field_bytes)
@@ -107,13 +86,23 @@ fn prepare_dimble_field(
10786
vr,
10887
inline_binary: None,
10988
} => {
110-
match value.as_slice() {
111-
[] if vr == b"SQ" => Ok(HeaderField::SQ(vec![])),
112-
[] => panic!("empty value"),
113-
[DicomValue::SeqField(seq)] => {
114-
let sq_header_field_map =
115-
prepare_dimble_fields(seq, data_bytes, pixel_array_safetensors_path)?;
116-
Ok(HeaderField::SQ(vec![sq_header_field_map]))
89+
if value.is_empty() {
90+
if vr == b"SQ" {
91+
return Ok(HeaderField::SQ(vec![]));
92+
}
93+
panic!("empty value");
94+
}
95+
96+
match value {
97+
DicomValue::SeqField(seqs) => {
98+
let seqs = seqs
99+
.iter()
100+
.map(|seq| {
101+
prepare_dimble_fields(seq, data_bytes, pixel_array_safetensors_path)
102+
})
103+
.collect::<InnerResult<_, _>>()?;
104+
105+
Ok(HeaderField::SQ(seqs))
117106
}
118107
dicom_values => {
119108
// call a function to handle this
@@ -335,29 +324,23 @@ mod tests {
335324
{
336325
let field = ir.get("00080005").expect("expected 00080005 to exist");
337326
assert_eq!(field.vr, *b"CS");
338-
let value: Vec<_> = field
327+
let value = field
339328
.value
340-
.iter()
341-
.map(|v| match v.as_slice() {
342-
[DicomValue::String(s)] => s,
343-
_ => panic!("expected only strings"),
344-
})
345-
.collect();
329+
.as_ref()
330+
.unwrap()
331+
.to_string_ref()
332+
.expect("expected only strings");
346333
assert_eq!(value, ["ISO_IR 100"])
347334
}
348335
{
349336
let field = ir.get("00080008").expect("expected 00080008 to exist");
350337
assert_eq!(field.vr, *b"CS");
351-
let value: Vec<_> = field
338+
let value = field
352339
.value
353340
.as_ref()
354341
.unwrap()
355-
.iter()
356-
.map(|v| match v {
357-
DicomValue::String(s) => s,
358-
_ => panic!("expected only strings"),
359-
})
360-
.collect();
342+
.to_string_ref()
343+
.expect("expected only strings");
361344
assert_eq!(value, ["ORIGINAL", "PRIMARY", "OTHER"]);
362345
}
363346
{
@@ -368,17 +351,14 @@ mod tests {
368351
{
369352
let field = ir.get("00100010").expect("expected 00100010 to exist");
370353
assert_eq!(field.vr, *b"PN");
371-
let value: Vec<_> = field
354+
let value = field
372355
.value
373356
.as_ref()
374357
.unwrap()
375-
.iter()
376-
.map(|v| match v {
377-
DicomValue::Alphabetic(a) => &a.alphabetic,
378-
_ => panic!("expected only alphabetic"),
379-
})
380-
.collect();
381-
assert_eq!(value, ["Doe^John"])
358+
.to_alphabetic_ref()
359+
.expect("expected only alphabetic");
360+
let value = value.iter().map(|x| &x.alphabetic).collect::<Vec<_>>();
361+
assert_eq!(value, &["Doe^John"])
382362
}
383363

384364
Ok(())

0 commit comments

Comments
 (0)