DICOM naive datetime retrieval
tomhampshire opened this issue · 6 comments
The primitive to_datetime
methods all expect a default, fixed time-offset to be passed, which is then applied to the resulting, timezone-aware datetime struct.
The DICOM standard allows for datetime VRs to have an optional suffix for offset from Coordinated Universal Time. In this case, it shouldn't be necessary to pass a fixed time-offset to the methods that retrieve datetime data.
It would be very helpful if, for a particular datetime primitive, you could retrieve a time-zone aware datetime if it exists, or a naive datetime if not.
Thank you for reporting. It is true that having to pass around a time offset to use by default is a bit unergonomic.
We happen to already have a DICOM date-time type that supposedly admits missing components, but there was probably an oversight when the time offset was made mandatory anyway, and this parameter was inherited from the previous revision. I believe that making DicomDateTime
accepting a missing time offset is the way to go. This would allow you to retrieve it without requiring a default offset, and they can be turned into naive date and time values all the same.
This would be a good change to make for the next release, as we are preparing for 0.7.0. Let me know if you would be interesting in pursuing this!
Hi - thanks for getting back to me!
I agree, having DicomDateTime
with a structure similar to:
pub struct DicomDateTime {
date: DicomDate,
time: Option<DicomTime>,
offset: Option<FixedOffset>,
}
(i.e. with an optional offset), then being able to access a chrono::NaiveDateTime
if offset
is None
, or a chrono::DateTime
if offset
is Some
.
I would be very interested in pursuing this. Accurate times are quite important for us!!
I'm going to roll with something like this, for the time being, but it feels a bit... dirty...
use crate::dicom_objects::instance::DicomInstance;
use dicom::core::DataElement;
use dicom::dictionary_std::tags;
use dicom::core::chrono::{prelude::*, DateTime};
const OFFSET_SECONDS: i32 = -1;
enum AnyDateTime {
TimezoneAware(DateTime<FixedOffset>),
Naive(NaiveDateTime),
}
fn convert(element: &DataElement) -> Result<AnyDateTime, Box<dyn std::error::Error>> {
let default_offset: FixedOffset = FixedOffset::east_opt(OFFSET_SECONDS).unwrap();
let dicom_date_time = element.to_datetime(default_offset.clone())?;
// Check to see if the default offset has been used. We can assume that there is no
// timezone information in this case, and we return a naive equivalent.
// If the offset has changed, we return a timezone aware value.
if dicom_date_time.offset().eq(&default_offset) {
Ok(AnyDateTime::Naive(
dicom_date_time.to_chrono_datetime()?.naive_local(),
))
} else {
Ok(AnyDateTime::TimezoneAware(
dicom_date_time.to_chrono_datetime()?,
))
}
}
I see ...
How about the DicomDateTime would impl:
.is_naive()_-> bool
or
.has_time_zone() -> bool
.to_naive_datetime -> chrono::NaiveDateTime
(would always work)
.to_datetime -> Option<chrono::DateTime<FixedOffset>>
I'm starting to look for a solution to this.
@tomhampshire In the meantime, your workaround contains one issue:
let dicom_date_time = element.to_datetime(default_offset.clone())?;
....
dicom_date_time.to_chrono_datetime()?.naive_local()
As a DicomDateTime
s precision cannot be known, to_chrono_datetime() might fail, if the value stored is not precise.
It's an inherent feature of these date / time values.
You will get the same result by calling dicom_date_time.exact()
.
You can check if the value is precise by `dicom_date_time.is_precise() and then proceed to an exact value.
If not precise, you only can retrieve a DateTimeRange
, see AsRange
#trait.
Thanks - yes, I came across this problem... I have been using: dicom_date_time.earliest()?.naive_local()
I'm not too concerned about the precision for my case, so I believe this should be fine.