-
Notifications
You must be signed in to change notification settings - Fork 34
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
Adding Images.jl support #16
Comments
Ping @timholy on this subject. My point of view is the following: I would like it if we could read a nifti file and the result will be an One issue I am unclear about is how the metadata should be presented. Will the resulting image object look similar if we load a Nifti or a DICOM file? Do we perform some "normalization" on the parameters? The end goal from my perspective should be
There can still be something like |
@Tokazama, oh dear, sorry, those docs are ancient. I didn't even realize they were still part of the repository. The Images.jl homepage does link to https://juliaimages.github.io/latest/ which is current, but we should delete those docs. As @tknopp says, using an ImageMetadata (which is basically the same thing as what you were referencing) could fix the issue. Normalization will be important if you want multiple file formats to recognize the format. The only format that returns consistent ImageMetadata now is NRRD, so perhaps check that for hints. |
I've been going back and forth in my head a lot on this lately and I think the thing to do would be something like a
I don't think we should even worry about tracking those fields that are unused in the NIfTI1 standard (eg, glmax, glmin, etc) since when writing the image those will probably just be given default values. So some I imagine implementing something similar for DICOMs would result in essentially identical structure for the |
Assuming there's lots of Nifti/DICOM-specific stuff in the header, I think that's a really good plan. If one wants to then write it out as a DICOM one could use |
Do we really require a dedicated type? Of course dicom and nifti will have additional metadata but having everything resulting in an ImageMetadata object will make the handling much easier from the user perspective. We should introduce required parameters that are normalized when loading and denormalized when saving. This concept for nifti, dicom, nrrd, vtk, and further formats would be awesome. |
Hmm, you're right, perhaps run-time checking is a better idea: there's nothing performance-sensitive about testing whether a given image is from a particular file type (you're not checking that on a pixel-by-pixel basis). What about the following standard for the ImageMetadata properties: Dict{String,Any} with 2 entries:
"fileformat" => "Nifti"
"fileheader" => Dict{String,Any}("niftifield1"=>'a',"niftifield2"=>2) Of course you could extract additional info into other named properties, and perhaps some of these could become semi-standard. For biomedical image formats, I highly recommend leveraging julia> using TestImages
julia> img = testimage("mri");
julia> print(summary(img))
3-dimensional AxisArray{ColorTypes.Gray{FixedPointNumbers.Normed{UInt8,8}},3,...} with axes:
:P, 0:1:225
:R, 0:1:185
:S, 0:5:130
And data, a 226×186×27 Array{Gray{N0f8},3} with eltype ColorTypes.Gray{FixedPointNumbers.Normed{UInt8,8}} This image is represented in PRS coordinates, hence the naming of the axes. And note the voxel separations encoded in the ranges. |
ping @hofmannmartin who is working together with me on this concept for a different format (MDF). I may also do this for the ISMRMRD format. |
I don’t think it would really require a dedicated type. I do think there should be a more modular framework that allows reading just header data. It's not uncommon to want some info from the header before importing the entire image array. It would also make it easier for other packages to build on NIfTI.jl. I imagine that some would like GIfTI, CIfTI, etc support. This is essentially NIfTI with some other stuff in the extension. |
NRRD supports |
Maybe an "ImageFormats.jl" package is in order. Either this could bundle everything together ( inclusive testing conversions) or it could be the basis. |
I like this. We have three categories:
|
It already exists: help?> spacedirections
search: spacedirections
spacedirections(img) -> (axis1, axis2, ...)
Return a tuple-of-tuples, each axis[i] representing the displacement vector between adjacent pixels along spatial axis i of the image array, relative to some external coordinate system ("physical coordinates").
By default this is computed from pixelspacing, but you can set this manually using ImagesMeta.
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
spacedirections(img)
Using ImageMetadata, you can set this property manually. For example, you could indicate that a photograph was taken with the camera tilted 30-degree relative to vertical using
img["spacedirections"] = ((0.866025,-0.5),(0.5,0.866025))
If not specified, it will be computed from pixelspacing(img), placing the spacing along the "diagonal". If desired, you can set this property in terms of physical units, and each axis can have distinct units. |
It looks like a lot of the structure for a standardized format is in place, but I'm not entirely sure of the right way to go about conforming to this. This has been my thought process:
I'm sure most of what's necessary to move forward is in place and additional clarity will arise as more unique imaging formats are implemented. However, it's not entirely clear what is necessary to be Image library "compliant" or if that's even supposed to be a thing. |
ImageCore = conversions/reinterpretations of values + frequently-used traits You shouldn't need anything higher in the stack than this. Here are ImageMetadata's requirements. No ImageFiltering/Images/etc. |
@Tokazama: Tim has designed Images "version 2" two years ago exactly in such a way that the dependencies are minimalistic. Don't get blended from the number of packages involved, this is more a sign of good modularity. Actually this is why I want to push the discussion into making "ImageMetadata" standard format for storing complex files like nifti and dicom. I don't have the bandwidth to contribute much code right now but it would be absolutely awesome if you could play with the idea for NiFTi.jl. I can give you rights such that you can work on a branch. For the record, here (https://github.com/MagneticParticleImaging/MPIFiles.jl/blob/master/src/Image.jl) you can find some code where I have the correspondence (ImageMetadata <-> MDF File). MDFs is a domain specific image format that I have developed. |
I'd be happy to put a bit of time into that. |
@Tokazama you should now have rights to make changes to this package. Best thing for new ideas is creating a branch and working there. |
tldr: I'm working on this again. It's not ready yet but it should be ready for heavy use very soon. Apologies for the long wait on this. Classes and projects put this on hold for a while. Here are some of the major changes in the most recent update and what the reasoning behind them were.
I still need to put together proper tests and fix a lot of the documentation, but I should actually be able to work on this again now that I have a bit more time. |
Great progress. Isn't NamesDims.jl just an alternative to AxisArrays.jl? The later also encodes the dim name with zero cost abstraction. I am currently using NIfTI.jl here |
NamedDims.jl is supposed to be the future implementation of NamesDims.jl and have zero cost at compile time (see JuliaCollections/AxisArraysFuture#1 (comment)). I'm not sure if this will happen anytime soon, but the functionality I'd like to get out of it isn't necessary for most use cases, so that's a minor issue for now. The goal is to have a generic interface for any |
Ok, I missed that new direction for AxisArrays. Whatever is used in the end. Most important is that we agree on a single standard, so that this remains compatible throughout different packages. Of what type should the |
Yeah, |
I figured there needed to be an update on this as my progress has been embarrassingly slow. Fortunately, I mostly just have to write a couple papers and work on this over the next two weeks. I actually was using my branch pretty heavily at one point (feeding thousands of images into a Flux model), so I don't expect there to be any major issues for people trying to do their day to day stuff. I've also enlisted a physics guy to stress test it with some multi-gigabyte DTI data he's working on. Hopefully, this will cover any major blind spots I may have. I'm going go over my plan moving forward on this as it appears some people may be waiting for me to actually get into gear and do what I actually said I'd do with this :P. I've ordered it according to priority so others may have an idea of when to expect things.
I expect to finish the first one fairly soon (weeks). I have no time table for the second or third one but I need them for several other projects that are coming up this next month. So if they don't get done soon that there will be some alternative strategy that fulfills a similar role. |
Hey, @Tokazama. Maybe it makes sense to chunk this a bit. I think 1. + some minimal form of documentation would be most important since it's the interface that people will rely on. Multithreading, 2. and 3. could perfectly follow afterwards. If you are fine with the interface, tests are passing just go ahead merge and release. |
I agree. I want to get the first out the door ASAP. I'm putting together the BIDS stuff here for now. Right now I'm just outlining everything so that it doesn't have to be rewritten every time a new format is supported. I don't plan to actual finish any more than the MRI relevant specification before getting the NIfTI branch ready to go. |
I am sure you have seen this. If it helps, you could add a detection function to FileIO and then use its |
Yes I have. And that's useful for the vast majority of cases, but there's a whole slew of CIfTI formats that require "somename.unique_variant.nii" so it requires an additional parsing of the "unique_variant" portion in addition to detecting the "nii". This also tells us it's likely a NIfTI-2 format because CIfTI is exclusively NIfTI-2 format. There are also unique combinations where if you have a certain set of extension then you expect to have another extension type file in the same folder (kind of like how Analyze requires a ".hdr" and ".img"). It's not something that will hold me up on getting the first release out the door too much, but I need to at least have a logical way to change how header data is handled based on the file extension. Especially the "extension" of the NIfTI file between the raw data and header. That stuff get's a little crazy. |
BTW. My latest stuff is nearly ready to be merged to my branch and once those tests are passing I'll merge. The only hold up at this point is getting NamedDims, AxisArrays, etc. compatibility finalized. |
So we can look forward to merging in 2021? 😄 I should engage a bit more with that process, it's clearly important. So little time though... |
I'm finishing up a pretty thorough write up on how to move that ahead that I'll post to relevant channels (hopefully today). I have to finish a fairly important analysis today, but once that's finished I'll push stuff to my branch. It probably won't work outside of my local packages without a the array stuff in place, but at least people could take a look at the method names and syntax so that we're sure they gel right with everyone. |
Hey @tanmaykm, One of the prime reason I am looking forward to your work being merged is that I would like to get rid of a custom Nifti writer that I have once written. I have moved the (ugly) code to a public location: https://github.com/tknopp/ImageUtils.jl/blob/master/src/AnalyzeNifti.jl I now have some questions. My Type is an https://github.com/tknopp/ImageUtils.jl/blob/master/src/Arrays.jl#L10 The dimensions might have a unit or not. So now the question is: How can I write such a type to a nifti using your new API? Next question: My export function also allows to write colorized Even if your branch is not feature complete, it would be great if we could merge this and then move on from there. |
I'll push something to my branch by the end of the night. It's likely not going to work because I need to get some other packages working with it first, but it should be a good place to start. In other words most of what I have for NIfTI.jl is ready but its dependencies aren't.
The biggest changes you'll see overall are that I'm trying to assign a dedicated trait method (similar to Also, thank you for those links. One of the biggest problems is that I think there should be a more established set of properties among MRI images if we want Julia and MR to really take off. The problem is that I've spent most of my time working on the image analysis side of things and I only know from collaboration and preprocessing pipelines what goes into the image acquisition. It's entirely possible that I miss some very useful ways of interacting with images. |
yes, it is space and time. Its important to get this into a canonical form before writing. If an axis has no unit but is spatial (x,y,z) then it would be great if you would consider this to be meters, time should be seconds.
That sounds great. But I a wondering: what will the object look like when I am reading a NIfTI file? Will it be a nifty specific type? If yes, will there be a conversion into some generic type (ImageMeta) possible?
There was
While I think a proper trait system is nice, my impression is that we at some point really want a file-independent type which you convert to. if you do image processing you will load the DICOM / Nifti file and will then want to forward that to you image pipeline. It makes me somewhat nervous when I never know what the concrete type is.
I am coming from the other side, I am doing image reconstruction and want to store things in the proper file format. Since you have some MR background you might be familiar with the ISMRMRD file format. In my package "MRIReco.jl" I have implemented full support for but still need something in which to store the reconstruction result. Wouldn't it be great if we could just use "DICOM.jl" and "NiFTI.jl" for that? If we have reached that state we would have an awesome Julia story here. |
I apologize for the terrible response time on this. I was hoping to get the array interface resolved but I think there was some loss of momentum in the community on that. I'll just get AxisArrays working for now. As for concerns about the trait system and knowing exactly what will be returned I have started on this JuliaNeuroscience/NeuroCore.jl#1. It probably won't match perfectly with ISMRMRD and I know there are differences from DICOM (mostly milliseconds to seconds). I've taken a look at your MRIReco.jl package and it differs in breaking properties into individual parameters instead of a dedicated structure for something like acquisition parameters. |
ISMRMRD is a raw data format so its not so relevant here. What I am looking for is an abstraction layer above DICOM / NiFTi. and ideally one has a bi-directional mapping between the files and its Julia representation. Is that what you are aiming for with NeuroCore.jl? Any reason this is specific to Neuro? |
We could of course just ignore BIDS as a whole and do our own thing, but the entire point of its creation was that too many groups kept doing that and it made research difficult. If at all possible I'd like to build on the work that the neuroscience community is beginning to adopt instead of reinventing the wheel. |
What is BIDS? Maybe I just did not get the background right here. |
Brain Imaging Data Standard. I wouldn't pay too much attention to another group trying to push their standard but a lot of software is beginning to incorporate it and a lot of the well funded projects now expect their software to follow it. It doesn't have complete coverage of neuroinformatic modalities but there are quite a few that are actively being worked on here. |
I'm trying to update this package and add Images.jl support. I've gotten to the point where I can get
include("./src/NIfTI.jl")
working on Julia 1.0. However, I'm not entirely sure how to proceed with incorporating Image.jl support.It's suggested here that image types have only two fields (
properties
anddata
). I figured I could proceed with one of the following strategiesheader
andextensions
field could be combined into theproperties
field so thatNIVolume <: AbstractImage
.AbstractImage
subtype.Thanks in advance
The text was updated successfully, but these errors were encountered: