Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default ValueReadStrategy #105

Closed
iberisoft opened this issue Feb 16, 2021 · 6 comments
Closed

Default ValueReadStrategy #105

iberisoft opened this issue Feb 16, 2021 · 6 comments
Labels

Comments

@iberisoft
Copy link

I am not very experienced with Rust so the issue might be explained easily.

Please explain how to switch from ValueReadStrategy::Preserved to ValueReadStrategy::Interpreted option. My issue is the following: I get a DICOM element of DS type however the element holds PrimitiveValue::Strs instead of PrimitiveValue::F64 because of using decode::read_value_preserved rather than decode::read_value inside the library based on what I have studied in the code.

My sample code is following:

fn print_value<I, P>(value: &DicomValue<I, P>) -> &str {
    if let Ok(_) = value.string() {
        return "string";
    }
    if let Ok(_) = value.uint16() {
        return "uint16";
    }
    if let Ok(_) = value.float64() {
        return "float64";
    }
    return "unknown"
}

I call print_value for each DataElement of a file. Almost any 16-bit image file contains the Window Center/Window Width tags which are float-point values. However, my code never returns float64.

@Enet4
Copy link
Owner

Enet4 commented Feb 17, 2021

Thank you for reaching out. Note that changing the parser reading strategy is not necessary for that use case. It only means that a conversion needs to take place after the parsing, which is the standard way of working with DICOM values at the moment. To treat a DS value as a floating point number:

// example
let val = DicomValue::new(dicom_value!(Strs, ["1300", "664 "]));

// converts strings automatically
let ww = val.to_float64()?;
// for multiple window levels, returns a vector
let ww = val.to_multi_float64()?;

The documentation on DicomValue has the full list of methods available. The ones named string, uint16 (and so on) only check their internal representation without conversion, whereas methods named to_* will try to convert the internal data if necessary. Note also that retrieving a simple enum value describing the value's type can be done through the method value_type.


Changing the value reading strategy requires creating the data set reader with the function DataSetReader.new_with_dictionary, then passing it manually to InMemDicomObject.from_element_source. This is not even sure to work well. The main reason for this is that any non-compliant value, such as a date or time value in another format, could prevent the parser from reading the rest of the data set. Context: #9 #10

@Enet4 Enet4 added the question label Feb 17, 2021
@iberisoft
Copy link
Author

Thank you @Enet4 for the detailed explanation. I got the idea of conversion on retrieval. Well... dates/times can be treated this way: storing strings but retrieving dates/times. However please explain why you did the same for floating-point values in contrast to integer values stored natively? Are you afraid of meeting a non-standard floating-point value, for example, with a decimal comma instead of a dot? Thank you for following up on this conversation!

@Enet4
Copy link
Owner

Enet4 commented Feb 18, 2021

However please explain why you did the same for floating-point values in contrast to integer values stored natively? Are you afraid of meeting a non-standard floating-point value, for example, with a decimal comma instead of a dot?

That would be a good point, and a justification to continue with this strategy, but the reasoning of this being the default is primarily to preserve the original representation: any textual value stays textual, thus preventing failures during the parsing phase and other oddities (such as decimal numbers losing precision when turned to a binary type).

This does not apply uniquely to dates/times and floating point values. If the VR is IS (e.g. for Instance Number), it will still be kept as a string. This is fine, as conversion is trivially done at a later stage through to_int or any of the other conversion methods.

@iberisoft
Copy link
Author

iberisoft commented Feb 19, 2021

OK @Enet4, thanks for clarifying the architecture.

Now I would like to share a real code converting DICOM elements into JSON values (some of them at least):

fn value_to_json<I, P>(element: &DataElement<I, P>) -> Option<JsonValue> {
    if element.header().len.get().unwrap() == 0 {
        return None;
    }
    println!("VR: {}", element.vr());
    let value = element.value();
    if value.primitive().is_some() {
        return match value.multiplicity() {
            0 => None,
            1 => single_value_to_json(value),
            _ => multi_value_to_json(value),
        };
    }
    println!("Skip {} as non-primitive element", element.header().tag);
    None
}

fn single_value_to_json<I, P>(value: &DicomValue<I, P>) -> Option<JsonValue> {
    if let Ok(value) = value.float64() {
        println!("float64");
        return Some(JsonValue::from(value));
    }
    if let Ok(value) = value.uint16() {
        println!("uint16");
        return Some(JsonValue::from(value));
    }
    if let Ok(value) = value.string() {
        println!("string");
        return Some(JsonValue::from(value));
    }
    None
}

fn multi_value_to_json<I, P>(value: &DicomValue<I, P>) -> Option<JsonValue> {
    if let Ok(value) = value.uint16_slice() {
        println!("uint16[]");
        return Some(JsonValue::from(value));
    }
    if let Ok(value) = value.float64_slice() {
        println!("float64[]");
        return Some(JsonValue::from(value));
    }
    if let Ok(value) = value.strings() {
        println!("string[]");
        return Some(JsonValue::from(value));
    }
    None
}

It traces the following for a file:

VR: UI
string
VR: UI
string
VR: DA
string
VR: TM
string
VR: CS
string
VR: LO
string[]
VR: CS
string
VR: UI
string
VR: UI
string
VR: US
uint16
VR: CS
string
VR: US
uint16
VR: US
uint16
VR: DS
string[]
VR: US
uint16
VR: US
uint16
VR: US
uint16
VR: US
uint16
VR: LT
string

Please note that value.float64_slice seems to return None for a multi-value DS-type element. I see a similar issue for single-value elements: value.float64 seems to return None for them. Could you explain why?

@Enet4
Copy link
Owner

Enet4 commented Feb 19, 2021

Please use to_multi_f64 instead. float64_slice is a non-converting method, and would only work for FD-type or OD-type elements.

@iberisoft
Copy link
Author

Please use to_multi_f64 instead. float64_slice is a non-converting method, and would only work for FD-type or OD-type elements.

Thank you, it seems to work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants