ENH: Reimplement GDCMSeriesFileNames on gdcm::IPPSorter/Scanner (drop deprecated SerieHelper)#6469
Open
hjmjohnson wants to merge 2 commits into
Open
Conversation
github-actions
Bot
added
type:Infrastructure
This comment was marked as resolved.
This comment was marked as resolved.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…twareConsortium#6467) Pin the observable contract of itk::GDCMSeriesFileNames before migrating its backend off the deprecated gdcm::SerieHelper onto gdcm::Scanner + gdcm::IPPSorter: single-series enumeration / grouping, geometric ordering that reconstructs a valid uniformly-spaced volume, GetInputFileNames returning the first series, and the Recursive flag controlling descent (which must be set before SetInputDirectory triggers the scan). These tests are backend-agnostic and must remain green across the swap.
Replace the deprecated gdcm::SerieHelper backend ("DO NOT USE ... temporary
solution for ITK migration from GDCM 1.x to 2.x") with the supported modern
API: gdcm::Directory for enumeration, gdcm::Scanner for series grouping, and
gdcm::IPPSorter for geometric (ImagePositionPatient on the slice normal)
ordering.
Behavior preserved on the in-tree suite (all 89 ITKIOGDCM tests pass,
including the series-reader and direction-cosine tests) and pinned by the
GDCMSeriesFileNames characterization tests added in the previous commit:
single-series enumeration / grouping, geometric ordering, GetInputFileNames
returning the first series, Recursive descent.
Intentional behavior changes:
- IPPSorter is strict: duplicate-IPP and gantry-tilt acquisitions FAIL to
sort; the input order is then left unchanged rather than fabricated.
Whether these need first-class support (and whether SerieHelper's permissive
strategy was correct) is tracked for discussion in InsightSoftwareConsortium#6468.
- AddSeriesRestriction now refines the series identifier (the documented and
example intent, e.g. "0008|0021"), instead of delegating to SerieHelper's
largely-inert file restriction list.
Part of InsightSoftwareConsortium#6467.
hjmjohnson
force-pushed
the
gdcmseriesfilenames-ippsorter
branch
from
19dc745 to
0e85890
Compare
github-actions
Bot
added
the
area:Documentation
Member
Author
|
@malaterre I have been looking at removing the "SerieHelper" dependency that is only kept around for ITK (according to the GDCM comments for SerieHelper). Your insights into the implications of the proposed change would be helpful. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reimplement
itk::GDCMSeriesFileNameson the supported modern GDCM API (gdcm::Directory+gdcm::Scanner+gdcm::IPPSorter), dropping the deprecatedgdcm::SerieHelper. Part of #6467.Why
gdcm::SerieHelperis documented by GDCM as deprecated backward-compat code — "DO NOT USE this class, it is only a temporary solution for ITK migration from GDCM 1.x to GDCM 2.x … Instead see ImageHelper or IPPSorter." This PR movesGDCMSeriesFileNamesto the supported API.What changed
gdcm::Directory(enumerate, honoring Recursive) →gdcm::Scanner(group by SeriesInstanceUID + detail tags; the series identifier replicatesSerieHelper::CreateUniqueSeriesIdentifier) →gdcm::IPPSorter(geometric ImagePositionPatient-on-normal ordering). Lazy parse with aTimeStampcache, so repeatedGetSeriesUIDs/GetFileNames/GetInputFileNamescalls no longer re-scan.GetInputFileNamesreturns the first series, Recursive descent. Green on both backends.Intentional behavior changes
AddSeriesRestrictionnow refines the series identifier (the documented and example intent, e.g."0008|0021"), instead of delegating to SerieHelper's largely-inert file-restriction list.Testing
All 89 ITKIOGDCM tests pass locally (series-reader, direction-cosine, compliance suites).
pre-commit run --all-filesclean. The vendoredgdcm::SerieHelperis untouched (still used by GDCM's own code); a follow-up may remove it once upstream GDCM drops it.