Skip to content

Legacy conversion revamp#388

Open
CPBridge wants to merge 138 commits into
v0.28.0devfrom
legacy_conversion_revamp
Open

Legacy conversion revamp#388
CPBridge wants to merge 138 commits into
v0.28.0devfrom
legacy_conversion_revamp

Conversation

@CPBridge

Copy link
Copy Markdown
Collaborator

Working PR to update legacy conversion, building on the work done by @afshinmessiah in #34. Supersedes #34

@fedorov

fedorov commented Jun 17, 2026

Copy link
Copy Markdown
Member

@CPBridge

CPBridge commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author
  • Sorting of image planes for legacy converted objects #5 Yes this is addressed to some extent, though there is room for further enhancement. Right now, by default it attempts to sort the inputs in two ways: first it checks if they are a regularly-sampled volume that can be sorted spatially, secondly (if spatial sorting fails), it just sorts by instance number and series number. Then it includes the DimensionIndexSequence to indicate how it sorted them. If both fail, it just includes the instances in the order they were passed, and no DimensionIndexSequence. You can also force this latter behaviour by passing include_dimension_index=False. In the future I would like to add the ability to specify custom dimension indices via a further parameter, but I would like to to do this in a further PR that enables this for other IODs too (e.g. a Segmentation of Parametric Map that has a time dimension, since at least two or three users have requested this.)
  • Errors in input data and legacy conversion #17 Not really. There are some specific pieces of logic to fix some problems in the input (particularly regarding ambiguous VRs, since failing to fix this results in a file that pydicom refuses to write out). But there is no attempt to implement a general patcher for all attributes that may be invalid in the input data. That would seem to be a big topic fraught with difficult decisions. Maybe we can talk about this in more detail.
  • Mapping of private into standard attributes as part of legacy enhanced MF generation #19 No. Standardising private attributes is not attempted in this PR. As discussed we can start working on this for a future release.
  • Sorting of attributes into Shared/PerFrame FGs #20 Yes, this is resolved by this PR. There is logic that detects which functional groups should be placed into the per-frame or shared functional groups sequences.
  • LegacyConvertedEnhancedMRImage does not copy Private Tag Creator #54 This is fixed by this PR: private tag creators are copied any time a private attribute is included.

@CPBridge

CPBridge commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Also, regarding #17 after reading the issue I'm still not clear on what sorts of issues you would propose to fix. Do you remember some specific classes of problem that it would be useful to solve? It may also have broader scope too, since all IODs in highdicom copy some number of attributes from their source images, the Legacy Converted Enhanced images just copy more of them. Should we try to implement general logic to patch attributes across the whole library, such that it would be used for SEG/PM/SR/PR etc?

@fedorov

fedorov commented Jun 17, 2026

Copy link
Copy Markdown
Member

@CPBridge thanks for the quick response, and sorry I definitely was way too sloppy in linking the issues without studying them in depth... I agree with your assessment, and I think this status update is super helpful. Let's just make sure those that are resolved are closed, and those other ones will need to be discussed/addressed separately.

@CPBridge

Copy link
Copy Markdown
Collaborator Author

Summary of this PR:

This PR contains a complete re-implementation of the legacy conversion classes, which much closer attention paid to small details and edge cases. It builds heavily on the work done by @afshinmessiah many years ago in #34, but refactors it to identify patterns that reduce boilerplate code, simplify, improve maintainability, and more closely follow pydicom and highdicom idioms.

Here is a non-exhaustive list of capabilities of this new implementation:

  • Automatic grouping of attributes into shared and per-frame functional groups according to whether they differ between instances
  • Correct handling of private attributes, including private creators
  • Encoding/decoding/transcoding to new transfer syntaxes, if requested, including with multiple workers
  • Conversion of big endian to little endian transfer syntaxes
  • Sorting and inclusion of the DimensionIndexSequence
  • Mapping of BodyPartExamined to AnatomicalRegionSequence
  • Conversion of Monochrome1 inputs to Monochrome2
  • Option to require that an input be a volume
  • Automatic population of the InStackPositionNumber if the input series is a volume
  • New section in the User Guide

The new implementation's API is backwards compatible with the old one, but there may be significant differences in the files produced

@CPBridge CPBridge added the enhancement New feature or request label Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LegacyConvertedEnhancedMRImage does not copy Private Tag Creator Sorting of attributes into Shared/PerFrame FGs

4 participants