Skip to content

Commit

Permalink
MAIN: Review comments
Browse files Browse the repository at this point in the history 
* Format object/src/mem.rs and object/src/lib.rs
* Change from u16 to u8, removing one error type
  • Loading branch information
@naterichman
naterichman committed Apr 22, 2024
1 parent 1d5ec8d commit 3d7e3b2
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 61 deletions.
26 changes: 10 additions & 16 deletions object/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,29 +290,23 @@ pub enum WriteError {
#[derive(Debug, Snafu)]
#[non_exhaustive]
pub enum PrivateElementError {
#[snafu(display("Element number must be less than 256, got {}", elem))]
InvalidElement{
elem: ElementNumber
},
#[snafu(display("Group number must be odd, found {}", group))]
InvalidGroup{
group: GroupNumber
},
InvalidGroup { group: GroupNumber },
#[snafu(display("Private creator {} not found in group {}", creator, group))]
PrivateCreatorNotFound {
creator: String,
group: GroupNumber
},
#[snafu(display("Private Creator {} found in group {}, but elem {} not found", creator, group, elem))]
PrivateCreatorNotFound { creator: String, group: GroupNumber },
#[snafu(display(
"Private Creator {} found in group {}, but elem {} not found",
creator,
group,
elem
))]
ElementNotFound {
creator: String,
group: GroupNumber,
elem: ElementNumber
elem: u8,
},
#[snafu(display("No space available in group {}", group))]
NoSpace{
group: GroupNumber
}
NoSpace { group: GroupNumber },
}

/// An error which may occur when looking up a DICOM object's attributes.
Expand Down
124 changes: 79 additions & 45 deletions object/src/mem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,14 @@ use crate::ops::{
};
use crate::{meta::FileMetaTable, FileMetaTableBuilder};
use crate::{
AccessByNameError, AccessError, AtAccessError, BuildMetaTableSnafu, CreateParserSnafu, CreatePrinterSnafu, DicomObject, FileDicomObject, InvalidElementSnafu, InvalidGroupSnafu, MissingElementValueSnafu, MissingLeafElementSnafu, NoSpaceSnafu, NoSuchAttributeNameSnafu, NoSuchDataElementAliasSnafu, NoSuchDataElementTagSnafu, NotASequenceSnafu, OpenFileSnafu, ParseMetaDataSetSnafu, PrematureEndSnafu, PrepareMetaTableSnafu, PrintDataSetSnafu, PrivateCreatorNotFoundSnafu, PrivateElementError, ReadError, ReadFileSnafu, ReadPreambleBytesSnafu, ReadTokenSnafu, ReadUnsupportedTransferSyntaxSnafu, UnexpectedTokenSnafu, WithMetaError, WriteError
AccessByNameError, AccessError, AtAccessError, BuildMetaTableSnafu, CreateParserSnafu,
CreatePrinterSnafu, DicomObject, ElementNotFoundSnafu, FileDicomObject, InvalidElementSnafu,

Check failure on line 58 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Build (Windows) 

unresolved import `crate::InvalidElementSnafu`

Check failure on line 58 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Test (default) (stable) 

unresolved import `crate::InvalidElementSnafu`

Check failure on line 58 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Check (macOS) 

unresolved import `crate::InvalidElementSnafu`

Check failure on line 58 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Test (default) (beta) 

unresolved import `crate::InvalidElementSnafu`
InvalidGroupSnafu, MissingElementValueSnafu, MissingLeafElementSnafu, NoSpaceSnafu,
NoSuchAttributeNameSnafu, NoSuchDataElementAliasSnafu, NoSuchDataElementTagSnafu,
NotASequenceSnafu, OpenFileSnafu, ParseMetaDataSetSnafu, PrematureEndSnafu,
PrepareMetaTableSnafu, PrintDataSetSnafu, PrivateCreatorNotFoundSnafu, PrivateElementError,
ReadError, ReadFileSnafu, ReadPreambleBytesSnafu, ReadTokenSnafu,
ReadUnsupportedTransferSyntaxSnafu, UnexpectedTokenSnafu, WithMetaError, WriteError,
};
use dicom_core::dictionary::{DataDictionary, DataDictionaryEntry};
use dicom_core::header::{ElementNumber, GroupNumber, HasLength, Header};

Check failure on line 67 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Build (Windows) 

unused import: `ElementNumber`

Check failure on line 67 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Test (default) (stable) 

unused import: `ElementNumber`

Check failure on line 67 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Check (macOS) 

unused import: `ElementNumber`

Check failure on line 67 in object/src/mem.rs

View workflow job for this annotation

GitHub Actions / Test (default) (beta) 

unused import: `ElementNumber`
Expand Down Expand Up @@ -366,8 +373,7 @@ where

// read rest of data according to metadata, feed it to object
if let Some(ts) = ts_index.get(&meta.transfer_syntax) {
let mut dataset =
DataSetReader::new_with_ts(file, ts).context(CreateParserSnafu)?;
let mut dataset = DataSetReader::new_with_ts(file, ts).context(CreateParserSnafu)?;

Ok(FileDicomObject {
meta,
Expand Down Expand Up @@ -451,8 +457,7 @@ where

// read rest of data according to metadata, feed it to object
if let Some(ts) = ts_index.get(&meta.transfer_syntax) {
let mut dataset =
DataSetReader::new_with_ts(file, ts).context(CreateParserSnafu)?;
let mut dataset = DataSetReader::new_with_ts(file, ts).context(CreateParserSnafu)?;
let obj = InMemDicomObject::build_object(
&mut dataset,
dict,
Expand Down Expand Up @@ -660,7 +665,7 @@ where
Err(super::AccessError::NoSuchDataElementTag { .. }) => Ok(None),
}
}

/// Get a particular DICOM attribute from this object by tag.
///
/// If the element does not exist,
Expand Down Expand Up @@ -698,26 +703,41 @@ where
}
}

fn find_private_creator(&self, group: GroupNumber, creator: &str) -> Option<&Tag>{
fn find_private_creator(&self, group: GroupNumber, creator: &str) -> Option<&Tag> {
let range = Tag(group, 0)..Tag(group, 0xFF);
for (tag, elem) in self.entries.range(range) {
// Private Creators are always LO
// https://dicom.nema.org/dicom/2013/output/chtml/part05/sect_7.8.html
if elem.header().vr() == VR::LO && elem.to_str().unwrap_or_default() == creator {
return Some(tag)
return Some(tag);
}
}
None
}

pub fn private_element(&self, group: GroupNumber, creator: &str, element: ElementNumber) -> Result<&InMemElement<D>, PrivateElementError> {
ensure!(element < 0xFF, InvalidElementSnafu{elem: element});
let tag = self.find_private_creator(group, creator)
.ok_or(PrivateCreatorNotFoundSnafu{group, creator: creator.to_string()}.build())?;

let element_num = (tag.element() << 8) | element;
Ok(self.get(Tag(group, element_num))
.ok_or(PrivateElementError::ElementNotFound{group, creator: creator.to_string(), elem: element})?)
pub fn private_element(
&self,
group: GroupNumber,
creator: &str,
element: u8,
) -> Result<&InMemElement<D>, PrivateElementError> {
let tag = self.find_private_creator(group, creator).ok_or(
PrivateCreatorNotFoundSnafu {
group,
creator: creator.to_string(),
}
.build(),
)?;

let element_num = (tag.element() << 8) | (element as u16);
self.get(Tag(group, element_num)).ok_or(
ElementNotFoundSnafu {
group,
creator: creator.to_string(),
elem: element,
}
.fail()?,
)
}

/// Insert a data element to the object, replacing (and returning) any
Expand All @@ -737,43 +757,39 @@ where
self.invalidate_if_charset_changed(elt.tag());
self.entries.insert(elt.tag(), elt)
}

pub fn put_private_element(
&mut self,
group: GroupNumber,
creator: &str,
element: ElementNumber,
element: u8,
vr: VR,
value: PrimitiveValue
) -> Result<Option<InMemElement<D>>, PrivateElementError>{
ensure!(element < 0xFF, InvalidElementSnafu{elem: element});
ensure!(group % 2 == 1, InvalidGroupSnafu{group});
value: PrimitiveValue,
) -> Result<Option<InMemElement<D>>, PrivateElementError> {
ensure!(group % 2 == 1, InvalidGroupSnafu { group });
let private_creator = self.find_private_creator(group, creator);
if let Some(tag) = private_creator{
if let Some(tag) = private_creator {
// Private creator already exists
let tag = Tag(group, tag.element() << 8 | element);
return Ok(self.put_element(DataElement::new(tag, vr, value)))

let tag = Tag(group, tag.element() << 8 | (element as u16));
return Ok(self.put_element(DataElement::new(tag, vr, value)));
} else {

// Find last reserved block of tags.
let range = Tag(group, 0)..Tag(group, 0xFF);
let last_entry = self.entries.range(range).rev().next();
let next_available = match last_entry {
Some((tag, _)) => tag.element() + 1,
None => 0x01
None => 0x01,
};
if next_available < 0xFF {
// Put private creator
let tag = Tag(group, next_available);
self.put_str(tag, VR::LO, creator);

// Put private element
let tag = Tag(group, next_available << 8 | element);
return Ok(self.put_element(DataElement::new(tag, vr, value)))

let tag = Tag(group, next_available << 8 | (element as u16));
return Ok(self.put_element(DataElement::new(tag, vr, value)));
} else {
return NoSpaceSnafu{group}.fail()
return NoSpaceSnafu { group }.fail();
}
}
}
Expand Down Expand Up @@ -1982,10 +1998,7 @@ mod tests {
use byteordered::Endianness;
use dicom_core::chrono::FixedOffset;
use dicom_core::value::{DicomDate, DicomDateTime, DicomTime};
use dicom_core::{
dicom_value,
header::DataElementHeader,
};
use dicom_core::{dicom_value, header::DataElementHeader};
use dicom_encoding::{
decode::{basic::BasicDecoder, implicit_le::ImplicitVRLittleEndianDecoder},
encode::{implicit_le::ImplicitVRLittleEndianEncoder, EncoderFor},
Expand Down Expand Up @@ -3684,7 +3697,7 @@ mod tests {
}

#[test]
fn private_elements(){
fn private_elements() {
let mut ds = InMemDicomObject::from_element_iter(vec![
DataElement::new(
Tag(0x0009, 0x0010),
Expand All @@ -3707,18 +3720,39 @@ mod tests {
"CREATOR 1",
0x01,
VR::DS,
PrimitiveValue::Str("1.0".to_string())
).unwrap();
PrimitiveValue::Str("1.0".to_string()),
)
.unwrap();
ds.put_private_element(
0x0009,
"CREATOR 4",
0x02,
VR::DS,
PrimitiveValue::Str("1.0".to_string())
).unwrap();
assert_eq!(ds.private_element(0x0009, "CREATOR 1", 0x01).unwrap().value().to_str().unwrap(), "1.0");
assert_eq!(ds.private_element(0x0009, "CREATOR 4", 0x02).unwrap().value().to_str().unwrap(), "1.0");
assert_eq!(ds.private_element(0x0009, "CREATOR 4", 0x02).unwrap().header().tag(), Tag(0x0009, 0x1202));

PrimitiveValue::Str("1.0".to_string()),
)
.unwrap();
assert_eq!(
ds.private_element(0x0009, "CREATOR 1", 0x01)
.unwrap()
.value()
.to_str()
.unwrap(),
"1.0"
);
assert_eq!(
ds.private_element(0x0009, "CREATOR 4", 0x02)
.unwrap()
.value()
.to_str()
.unwrap(),
"1.0"
);
assert_eq!(
ds.private_element(0x0009, "CREATOR 4", 0x02)
.unwrap()
.header()
.tag(),
Tag(0x0009, 0x1202)
);
}
}

0 comments on commit 3d7e3b2

Please sign in to comment.