DICOM naive datetime retrieval

[tomhampshire]

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.

[Enet4]

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!

[tomhampshire]

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!!

[tomhampshire]

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()?,
        ))
    }
}

[jmlaka]

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

[jmlaka]

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.

[tomhampshire]

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.