Default to None if voilut is not present#536
Conversation
Enet4
left a comment
There was a problem hiding this comment.
Thank you for looking into this. Can you provide a sample file that tripped this line? voi_lut_function should not error when the attribute is missing entirely, but rather when it has an invalid value. I suspect that this was caused by VOI LUT Function being present in the data set, but with an empty value.
At this time I would rather seek to increase the resilience of voi_lut_function against empty values (treating them as they were not present at all) than change places where the function is called.
Here is a test function that you can add to attribute::tests:
#[test]
fn get_voi_lut_function() {
use super::voi_lut_function;
let mut obj = FileDicomObject::new_empty_with_meta(
FileMetaTableBuilder::new()
.transfer_syntax(uids::EXPLICIT_VR_LITTLE_ENDIAN)
.media_storage_sop_class_uid(uids::ENHANCED_MR_IMAGE_STORAGE)
.media_storage_sop_instance_uid("2.25.145929179730251416957282651365760465911")
.build()
.unwrap(),
);
// Returns None if not present
assert_eq!(voi_lut_function(&obj).unwrap(), None);
obj.put(DataElement::new(
tags::VOILUT_FUNCTION,
VR::CS,
PrimitiveValue::Empty,
));
// Returns None if empty
assert_eq!(voi_lut_function(&obj).unwrap(), None);
obj.put(DataElement::new(tags::VOILUT_FUNCTION, VR::CS, "LINEAR"));
// Returns the value
assert_eq!(
voi_lut_function(&obj).unwrap(),
Some(vec!["LINEAR".to_string()])
);
obj.remove_element(tags::VOILUT_FUNCTION);
// per-frame functional groups
obj.put(DataElement::new(
tags::PER_FRAME_FUNCTIONAL_GROUPS_SEQUENCE,
VR::SQ,
DataSetSequence::from(vec![InMemDicomObject::from_element_iter([
DataElement::new(tags::VOILUT_FUNCTION, VR::CS, "LINEAR_EXACT"),
])]),
));
assert_eq!(
voi_lut_function(&obj).unwrap(),
Some(vec!["LINEAR_EXACT".to_string()])
);
}|
According to the standard it says the VOI LUT function is optional, and when not present it should default to LINEAR (well, actually the window center and window width should be calculated as LINEAR). What do think of how I updated the PR. It should default to LINEAR in case the tag is not present. |
Yeah, we could go in that direction for this function here. In that case we never return |
Enet4
left a comment
There was a problem hiding this comment.
I will approve this and defer optimizations to other pull requests. Thanks again! 👍
This is a similar fix as applied on #514