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

Common Image/Array Interface #62

Open
Tokazama opened this issue Aug 12, 2020 · 5 comments
Open

Common Image/Array Interface #62

Tokazama opened this issue Aug 12, 2020 · 5 comments

Comments

@Tokazama
Copy link
Member

I've been working on revamping a lot of the underlying components of JuliaImages for quite some time now (this rambling issue) and I'd like to see if I could assist with some things in DICOM.jl along the way.

I've put together some of this already in the docs for NeuroCore here, but there are basically four components:

  1. Raw array data: the image
  2. Dimension names: if it were an axial view the dimension names could be something like (:sagittal, :coronal).
  3. Axis keys: The actual pixels coordinates in millimeters along each axis
  4. Metadata: Some structure (typically a dictionary) that stores the non array specific data

I was hoping to get a minimal working example for this but I got a bit stuck on some of the details and I'm sure that depending on the modality the tags used to compose each part of this would be different, so here's an example where I fudged certain details with obnoxiously long method names.

function dcm_to_image(data::Dict{Tuple{UInt16,UInt16},Any}, wanted_names)
    raw_image = data[(0x7FE0, 0x0010)]
    dimension_names = get_dimension_names(data)
    xkeys = range(x_axis_start(data), step = data[(0x0028, 0x0030)][1], length= data[(0x0028, 0x0011)]) * Unitful.mm
    ykeys = range(y_axis_start(data), step = data[(0x0028, 0x0030)][2], length= data[(0x0028, 0x0011)]) * Unitful.mm

    # convert names to snake case symbols so they can be retreived like properties
    meta = Dict{Symbol,Any}()
    for x in wanted_names
        meta[Symbol(join(lowercase.(split(k)), "_"))] = DICOM.lookup(data, x)
    end

    return AxisIndices.NamedMetaAxisArray{dimension_names}(raw_image, (xkeys, ykeys), metadata=meta)
end

There are some other things we can do to make it easy to convert to color types also, but this is the basic idea. Also, if this ends up gaining some traction I have figured out ways to encode spatial information that should be compatible with JuliaImges, so rotation and linear transforms should be possible to include.

The ugliest part of this seems to be handling metadata. The metadata doesn't necessarily have to use Symbol keys, but 1) it allows doing image.kept_name and 2) it follows the norm in Julia of referring to dictionaries of labeled data using Symbol.

@notZaki
Copy link
Member

notZaki commented Aug 13, 2020

These would be nice additions for processing the data.
I don't know whether these features ought to be in DICOM.jl or in a different package.

The dcm_to_image function appears general enough to work on most modalities.
I think one of the (0x0028, 0x0011) should be (0x0028, 0x0010) instead---alternatively, the dimensions of raw_image could be used.

Regarding the metadata, we can address that issue at the source by using symbols as fieldnames in DICOM.jl. I opened a PR that does this.

@Tokazama
Copy link
Member Author

These would be nice additions for processing the data.
I don't know whether these features ought to be in DICOM.jl or in a different package.

DICOM is already a beast of a format to maintain, so I understand wanting to keep this package as simple as possible. Do you have a vision for how/where this sort of thing should be. Should it be in a package like DICOMImages.jl?

Regarding the metadata, we can address that issue at the source by using symbols as fieldnames in DICOM.jl. I opened a PR that does this.

This sounds great! Initially, I anticipated this being the most difficult to resolve.

@notZaki
Copy link
Member

notZaki commented Aug 15, 2020

You will have a better idea of whether the function(s) should be in a new package or if they can be included in AxisIndices.jl/NeuroCore.jl.

What are the plans for NIfTI.jl? Will it eventually return *AxisArrays or stay with NIVolume?

@Tokazama
Copy link
Member Author

NIfTI will definitely use a different array type in the future. The only reason I haven't pushed all my changes to github is because I need it to be compatible with JuliaImages and electrophysiology stuff (mostly generic time series stuff). I feel like I'm 99% of the way there; but you can see from this thread on discourse and the aforementioned issue to Images.jl that I'm going to have to work with some potentially hairy breaking changes and disagreements.

@notZaki
Copy link
Member

notZaki commented Aug 19, 2020

Major breaking changes are something I would prefer to avoid, but at the same time, it would be great if DICOM.jl was integrated with the rest of the imaging ecosystem.

One area we could work on is by defining all the subfunctions in dcm_to_image. For example, get_dimension_names will likely involved interpreting dcm.ImageOrientationPatient. These functions can be included in DICOM.jl and integrated with the test suite to make sure they work across different modalities.
Once those building blocks are in place, it would be trivial to convert the dicom data into an array type from AxisIndices/NeuroCore that is compatible with the rest of the ecosystem.

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

No branches or pull requests

2 participants