QIICR / lidc2dicom

Scripts for converting TCIA LIDC-IDRI collection derived data into standard DICOM representation from project-specific XML format.
24 stars 12 forks source link

Add highdicom implementation #7

Closed CPBridge closed 3 years ago

CPBridge commented 3 years ago

Hi @fedorov,

Adding an alternative version of the main script that uses the highdicom library, as discussed. This is mostly a drop-in replacement of the original, but not in all aspects, including (but not limited to):

I have also added a brief explanation in the README.

I am marking as a draft right now because this depends on functionality in this PR on highdicom. This will probably be merged in the next few days, but there's a small chance the public API may change between now and then, in which case I will update this PR. I will mark as ready for review when this is finalised.

There is one known issue currently: the SR documents produced may contain decimal strings that have more than 16 characters, which are invalid according to the standard. Some tools (including dcmtk's dsr2html, dsrdump, dsr2xml) will complain about this. The fix is already implemented in pydicom master branch, but is not released. When this highdicom PR is merged, which is currently blocked by the fact that the pydicom behaviour is not released, this error will automagically disappear without having to change the lidc2dicom script as long as an updated version of highdicom is used.

Tagging @hackermd for visibility.

fedorov commented 3 years ago

Was: "Chris, I am sorry for the delay in merging this. Thank you for this contribution!"

Happy to merge this as soon as you mark this as non-draft!

CPBridge commented 3 years ago

@fedorov you're welcome. The PR on highdicom blocking this has been merged, and I have marked this as ready

fedorov commented 3 years ago

Excellent! Thanks for the contribution!