Refactor itkimage2dcmSegmentation and itkimage2paramap to support "in memory" DCM sources.

[rfloca]

Status:
Currently the interfaces of Itkimage2dcmSegmentation and itkimage2paramap build up a dependency to DcmDataset as you pass a vector of DcmDataset* representing the dicom source images.

Proposal:
As far as I can see in the implementation of dcmqi only parts of the class interface is used that is directly inherited from DcmItem.
If this is correct and I am not mistaken this would mean that you could easily switch to the following signatures:

   static DcmDataset* itkimage2dcmSegmentation(vector<DcmItem*> dcmSourceItems,
                                               vector<ShortImageType::Pointer> segmentations,
                                               const string &metaData,
                                               bool skipEmptySlices=true);

   static DcmDataset* itkimage2paramap(const FloatImageType::Pointer &parametricMapImage, vector<DcmItem*> dcmSourceItems,
                                       const string &metaData);

It would not break existing code, but ensures the possibility to use the methods, when you have no dcm files but all informations you need. As it is e.g. in some of our use cases, where we have the values of the dcm tags at hand but not the original files. The change in the function signature would gurantee that one can directly populate DcmItems in memory and pass them instead of trying to get the files or "simulate" them by hacking temporary files with the needed information; because nothing DcmDataset-specific is used.

Have I missed something? What do you think.

If the change proposal is supported and there is reasoning against it, we could also provide a patch sooner then later, because it would ensure that we do not implement against implementation details that might change.

Looking forward to read your thoughts.

[fedorov]

Ralf, thank you for reaching out! I know you guys brought this use case several times, and I do want to support it. Over a year ago now, we started working on complete refactoring of dcmqi, to address this issue, among many others - see #226. Very unfortunately, we ran out of steam (and also got bogged down into the issues related to lacking experience with Git workflows while developing a branch collaboratively with Christian Herz...) although I thought we were quite close.

Let me think about this, I have not touched or looked at that code in a while!

[rfloca]

Hi Andrey,

thanks for your swift response. Also thanks for üointing out the other issue, looks realy interesting.
Let me know what you think about the proposed mitigation strategy (I think, if finished, the approach in #226 is far better and more sophisticated), as soon as you have time to think about it.
But before that, have a merry christmas.

[fedorov]

@rfloca I did not forget, sorry for not responding. I will try to get to look at it this week.

[fedorov]

@rfloca sorry again for the unacceptable delay...

I looked into your proposal, and I don't see any problem with what you are suggesting. I also looked into the refactoring effort that I mentioned earlier, and I can't tell you if or when I will get to it - it will be a significant effort to revive that PR. What you are suggesting is sensible, and I think should not be disruptive, so I would definitely welcome your patch with the functionality you proposed!

[rfloca]

@fedorov no worries. I was bussy myself (as you can see due to my delay)

Thanks for the feedback. I will make a pullrequest with a patch as soon as possible!

[michaelonken]

Just my 2 cents after quick-checking the current code:

It should be possible switching to DcmItem instead which is generally more "flexible" than using a DcmDataset. We could be able to refactor this quickly without problems.

However, I suggest to wait for the discussion regarding NA-MIC/ProjectWeek#881 before putting hands on the function signatures.

[rfloca]

@michaelonken Thanks for looking into that matter and confirming the assumption. I agree that we should wait for the results of the discussion and then see how to move forward. Looking forward to the exchange at the project week.

[fedorov]

I have no objection to changing the signature. I am not aware of any other tool that uses dcmqi API and not command line converters other than MITK, so I doubt it will cause much trouble.

[rfloca]

Hi everybody. I hope you are fine.

This problem came up again in some projects and the MITK mailing list. I would like to know what the status is here. If I remember correctly @michaelonken said in the VCon in January, that he could refactor it swiftly. But maybe it was already done and I missed it 🙈

[michaelonken]

Hi Ralf,

I've done this already in some branch (don't have it at hand right now) but our main problem is that we cannot use a newer DCMTK in dcmqi since it will then class with the DCMTK libraries brought in by ITK dependencies. This is currently under investigation (for some time now...). I don't know the current status, maybe @fedorov knows more.

[fedorov]

@jcfr @jadh4v do you have any updates on the status of fixing CMake issues and updating DCMTK in dcmqi and Slicer?

[rfloca]

Hi Ralf,

I've done this already in some branch (don't have it at hand right now) but our main problem is that we cannot use a newer DCMTK in dcmqi since it will then class with the DCMTK libraries brought in by ITK dependencies. This is currently under investigation (for some time now...). I don't know the current status, maybe @fedorov knows more.

@michaelonken Thanks for the information. From which version on DCMTK has problems with ITK? So far we witnessed no problems with an external DCMTK used in ITK and MITK to build upon.

[michaelonken]

I think we tried to link a new DCMTK (such as one of the current commits on master) by specifying it in the dcmqi build process.

When linking the linker complains that there are two versions of DCMTK libs around, e.g. dcmdata, one from the dcmqi link and one from (as fas I as could see) ITK, which brings its dependencies into the dcmqi link process.