JuliaHealth / DICOM.jl

Julia package for reading and writing DICOM (Digital Imaging and Communications in Medicine) files
MIT License
56 stars 21 forks source link

Feature: Access fields using dcm.field instead of dcm[tag"field"] #63

Closed notZaki closed 4 years ago

notZaki commented 4 years ago

The current fieldnames are strings because they use the name defined in the DICOM registry. These names contains spaces.

This PR changes the fieldnames into symbols by using the keyword defined in the same DICOM registry---these do not have spaces.

To avoid breaking existing code, fieldnames are still accepted as strings. These are converted to symbols by stripping any spaces. There are a few corner cases where this can fail, e.g. some names contain a parenthesis that is omitted in the keyword.

The main advantage of this change is that it will allow us to use dot syntax in the future, e.g. dcm.PixelData instead of dcm[(0x7FE0,0x0010)] or dcm[tag"Pixel Data"].

codecov[bot] commented 4 years ago

Codecov Report

Merging #63 into master will increase coverage by 0.69%. The diff coverage is 95.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #63      +/-   ##
==========================================
+ Coverage   88.12%   88.82%   +0.69%     
==========================================
  Files           1        2       +1     
  Lines         320      340      +20     
==========================================
+ Hits          282      302      +20     
  Misses         38       38              
Impacted Files Coverage Δ
src/DICOM.jl 88.16% <80.00%> (+0.03%) :arrow_up:
src/DICOMData.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9a48479...60d1238. Read the comment docs.

notZaki commented 4 years ago

Update: The current dcm_parse function returns a dictionary. The PR has added a new type called DICOMData which wraps the dictionary. This allows new ways of accessing the dicom data because getindex and getproperty can dispatch off the type.

With these changes, the dicom data can be accessed by dcm.fieldname or dcm[:fieldname] or dcm["fieldname"]. The previous approaches still continue to work, i.e. dcm[(0xHEX,0xHEX)] and dcm[tag"fieldname"] remain valid.

Tokazama commented 4 years ago

Looks good! Would it make sense to use an immutable dict for the package's look up table?

notZaki commented 4 years ago

I didn't know they were a thing. I'll give it a try.

notZaki commented 4 years ago

I tried using an immutable dict for dcm_dict and fieldname_dict along with combinations where one was immutable and the other wasn't. In each case, the ImmutableDict was slower.

According to the julia docs:

[ImmutableDict] is optimal for small dictionaries that are constructed over many individual insertions.

which might explain why the look up table doesn't benefit; it is a large table that is constructed once.