ENH: Ingest ITKIOScanco into Modules/IO#6266
Open
hjmjohnson wants to merge 137 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}
The space-padding loop iterated length times unconditionally, on top of the bytes already copied from source. PadString therefore wrote up to 2*length bytes into dest, overrunning the fixed-width header fields that follow it on every ISQ/AIM write.
CheckVersion called strcmp on a fixed-width 16-byte buffer that is not guaranteed to be null-terminated (CanReadFile reads 512 raw bytes from the file and passes the pointer in). The comparison could walk past the buffer's logical end into trailing header bytes, producing undefined behavior on arbitrary inputs. Use strncmp bounded by VersionDiskWidth, matching the existing CTDATA check.
…mats The else branch unconditionally returned fileType = 2 (AIMDATA_V020) for any header that failed both prior checks, so CanReadFile claimed every readable file as a Scanco file and the IO factory would self-nominate for unrelated inputs. Match AIMDATA_V020 explicitly via strncmp and let fileType remain 0 when no magic string matches.
ReadImageInformation called SetPixelType(SCALAR) unconditionally after the AIM branch had already called ParseAIMComponentType, which sets VECTOR for multi-component AIM data types (e.g. 0x00120003, 0x00060003). The trailing SetPixelType clobbered that decision, leaving downstream pipelines to treat multi-component AIM volumes as scalar.
WriteHeader's open call passed the local filename parameter, which is empty when the caller relied on a previously-set m_FileName (the case the surrounding check explicitly accepts). Use m_FileName, matching ReadHeader. Also correct "/n" to "\\n" in the error message.
add_definitions() leaks to every sibling target compiled in the remaining directory scope. Replace with target_compile_definitions on the ITKIOScanco library defined in src/CMakeLists.txt.
Member
Author
|
Addressed all 5 P1 + 1 P2 greptile findings in 6 BUG:/COMP: fixup commits appended to preserve merge topology. ITKTestingData PR #50 just merged; ExternalData cache should resolve within ~5 min, so Pixi-Cxx CI should rerun green. Commits
|
Member
Author
|
/azp run |
Member
Author
|
@greptileai review this draft — addressed all 6 findings in commits d4a96ec..ba4ba5b. Please confirm no regressions. |
…b 100MB) The IOScanco ISQ test input blob is 106,958,336 bytes, which exceeds GitHub's 100 MB hard limit for blobs in repositories. The CID content-link in ITKTestingData PR InsightSoftwareConsortium#50 produced only a zero-byte marker file because the actual content could not be pushed. Restore the original SHA512 content-link. ExternalData resolves this file via the data.kitware.com SHA512 mirror, which hosts the full blob at: https://data.kitware.com/api/v1/file/hashsum/sha512/<digest>/download (HTTP 200, Content-Length 106958336 — verified 2026-05-14.) The orphaned zero-byte CID marker on ITKTestingData gh-pages is removed in a companion PR.
Member
Author
|
Reverted Companion cleanup: InsightSoftwareConsortium/ITKTestingData#55 removes the orphaned zero-byte CID marker from The Baseline |
The C0004255.ISQ fixture is a 107 MB file hosted on data.kitware.com via SHA512 mirror (it exceeds GitHub's 100 MB single-file limit so cannot live in ITKTestingData). Tests that require this fixture are gated behind ITK_BUILD_BIG_DATA_TESTS (default OFF) and labelled with BigIO;RUNS_LONG, matching the pattern used by itkLargeTIFFImageWriteRead* tests in Modules/IO/TIFF. Default builds keep the smaller AIM (4 MB) fixtures, exercising the reader without the multi-hundred-MB download.
Two related changes that together unblock the Populate ExternalData Cache workflow on PR InsightSoftwareConsortium#6266: 1. Replace the ITK_BUILD_BIG_DATA_TESTS option with the established ITK_COMPUTER_MEMORY_SIZE > 5 gate (matches the TIFF itkLargeTIFFImageWriteReadTest* pattern in Modules/IO/TIFF). CI dashboards declare ITK_COMPUTER_MEMORY_SIZE=4.5 so the C0004255 tests are not registered on those agents. 2. Convert Baseline/C0004255.mha.cid -> .mha.sha512. The Populate ExternalData Cache GitHub Action scans **/*.cid across the whole source tree (regardless of CMake gating), so the .cid form forced the workflow to fetch a 67 MB blob that was never uploaded to ITKTestingData. Switching to .sha512 routes the fetch through data.kitware.com's hashsum mirror (which has the blob) and removes it from the cache-populate expected set.
The if(NOT ITK_SOURCE_DIR) branch (find_package(ITK) + include(ITKModuleExternal)) only executes when building the module standalone outside ITK. In-tree builds always take the itk_module_impl() else branch. Drop the dead branch following the cleanup pattern from InsightSoftwareConsortium#6272/InsightSoftwareConsortium#6279.
50 tasks
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 14, 2026
The Populate ExternalData Cache CI workflow scans every *.cid file and fails the build if any CID cannot resolve via the IPFS gateway list. The two new Strain baselines (LineLoadStrainLagrangian.mha, LineLoadStrainEulerian.mha) were only registered as zero-byte markers on ITKTestingData gh-pages and do not resolve through the gateways. Switch both baselines to the .sha512 content-link form so that ExternalData resolves them via the data.kitware.com hashsum mirror instead of the IPFS gateway list, matching the pattern used for IOScanco baselines in PR InsightSoftwareConsortium#6266.
Member
Author
|
/azp run |
…InsightSoftwareConsortium#56) Convert 1 .sha512 content-link to .cid form now that the blob is staged on the ITKTestingData gh-pages mirror via PR InsightSoftwareConsortium#56: Modules/IO/IOScanco/test/Baseline/C0004255.mha.cid The companion C0004255.ISQ remains as .sha512 (too large for gh-pages).
Member
Author
|
/azp run |
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
8 participants
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.
Ingest the ITKIOScanco remote module into
Modules/IO/IOScanco(Wave 2 of #6160), preserving upstream merge topology. Replaces the.remote.cmakereference and enablesModule_IOScancoin the configure-ci pixi task.What was ingested
Modules/IO/IOScanco/upstream/main(merge topology preserved peringest-merge-topology.md)Merge-topology preservation
The ingest was performed via
--allow-unrelated-historiesmerge of the filter-repo'd upstream module history. All upstream merges from the originalKitwareMedical/ITKIOScancorepository are preserved as two-parent merge commits;git blamewalks back through the merge into the original upstream contributors.Follow-on commits
COMP: Remove IOScanco .remote.cmake (in-tree)— drops the now-redundant remote-module manifest.ENH: Enable Module_IOScanco in configure-ci pixi task— surfaces the in-tree module in the CI configure step.ENH: Normalize C0004255.ISQ to CID content-link (now in ITKTestingData)— switches the test fixture from a.sha512URL reference to an ExternalData CID link.Test data (ITKTestingData)
The
C0004255.ISQtest fixture has been published to ITKTestingData via PR InsightSoftwareConsortium/ITKTestingData#50 (merged). CID:The ingest tip references this CID directly via ExternalData content-link, so the test resolves once the ITKTestingData submodule reference picks up the merged PR.