ENH: Ingest ITKMinimalPathExtraction into Modules/Filtering/MinimalPathExtraction#6271
Merged
hjmjohnson merged 91 commits intoMay 17, 2026
Conversation
github-actions
Bot
added
type:Infrastructure
Member
Author
|
@greptileai review this draft before I make it official |
1 similar comment
Member
Author
|
@greptileai review this draft before I make it official |
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}
This comment was marked as outdated.
This comment was marked as outdated.
Member
Author
|
/azp run |
dzenanz
approved these changes
May 14, 2026
63 tasks
Member
Author
|
/azp run |
1 similar comment
Member
Author
|
/azp run |
Member
Author
|
/azp run |
1 similar comment
Member
Author
|
/azp run |
hjmjohnson
force-pushed
the
ingest-MinimalPathExtraction
branch
from
352b1bb to
6c9ea35
Compare
Member
Author
|
Rewrote history with |
Member
Author
|
/azp run |
1 similar comment
Member
Author
|
/azp run |
hjmjohnson
force-pushed
the
ingest-MinimalPathExtraction
branch
from
b0372e2 to
0cd4791
Compare
Member
Author
|
History rewrite (filter-repo) to address ghostflow What was rewritten and whyGhostflow flagged three classes of issues in historical commits of this ingest branch:
Merge topology preserved: 127 commits, 36 merges, identical before and after. Tree at the new tip differs from the previous tip only in the 12 mode changes on the surviving Verification
Old tip: |
Member
Author
|
/azp run |
This check replaces default bodies of special member functions with = default;. The explicitly defaulted function declarations enable more opportunities in optimization, because the compiler might treat explicitly defaulted functions as trivial. Additionally, the C++11 use of = default more clearly expreses the intent for the special member functions.
Find and remove redundant void argument lists.
expression This check is responsible for using the auto type specifier for variable declarations to improve code readability and maintainability. The auto type specifier will only be introduced in situations where the variable type matches the type of the initializer expression. In other words auto should deduce the same type that was originally spelled in the source
The emptiness of a container should be checked using the empty() method instead of the size() method. It is not guaranteed that size() is a constant-time function, and it is generally more efficient and also shows clearer intent to use empty(). Furthermore some containers may implement the empty() method but not implement the size() method. Using empty() whenever possible makes it easier to switch to another container in the future. SRCDIR= #My local SRC BLDDIR= #My local BLD cd run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,readability-container-size-empty -header-filter=.* -fix
The mission of NumFOCUS is to promote open practices in research, data, and scientific computing. https://numfocus.org
The mission of NumFOCUS is to promote open practices in research, data, and scientific computing. https://numfocus.org
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
The ability to include either .h or .hxx files as header files required recursively reading the .h files twice. The added complexity is unnecessary, costly, and can confuse static analysis tools that monitor header guardes (due to reaching the maximum depth of recursion limits for nested #ifdefs in checking).
With updates to FastMarching in ITK, it is necessary to SetTargetPoints prior to setting TargetReachMode. Without this change, an exception is thrown in itkFastMarchingUpwindGradientImageFilter:: VerifyTargetReachedModeConditions() when called by SetTargetReachedModeToAllTargets() which is called by itkSpeedfunctionToPathFilter:: ComputeArrivalFunction()
Modules/Remote/MinimalPathExtraction/include/itkSpeedFunctionToPathFilter.hxx:239:51: warning: ignoring return value of 'bool itk::ImageBase<VImageDimension>::TransformPhysicalPointToContinuousIndex(const itk::Point<TCoordRep, VImageDimension>&, itk::ContinuousIndex<TIndexRep, VImageDimension>&) const [with TCoordRep = double; TIndexRep = double; unsigned int VImageDimension = 2]', declared with attribute 'nodiscard' [-Wunused-result]
…acro Added two new macro's, intended to replace the old 'itkTypeMacro' and 'itkTypeMacroNoParent'. The main aim is to be clearer about what those macro's do: add a virtual 'GetNameOfClass()' member function and override it. Unlike 'itkTypeMacro', 'itkOverrideGetNameOfClassMacro' does not have a 'superclass' parameter, as it was not used anyway. Note that originally 'itkTypeMacro' did not use its 'superclass' parameter either, looking at commit 699b66c, Will Schroeder, June 27, 2001: https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
Use static constexpr directly now that C++11 conformance is required by all compilers. :%s/itkStaticConstMacro *( *\([^,]*\),[ \_s]*\([^,]*\),\_s*\([^)]*\)) */static constexpr \2 \1 = \3/ge 'itkStaticConstMacro(name, type, value)' became unconditionally identical to 'static constexpr type name = value' with ITK commit aec9519 "ENH: Update compiler macros (InsightSoftwareConsortium#810)", maekclena, 7 May 2019. 'itkGetStaticConstMacro(name)' became unconditionally identical to '(Self::name)' with ITK commit 84e490b "Removing some outdated compiler conditionals", Hans Johnson, 31 July 2010. Most 'itkStaticConstMacro' calls were removed by ITK commit 5c14741
For the sake of code readability, a new 'CoordinateType' alias is added for each nested 'CoordRepType' alias. The old 'CoordRepType' aliases will still be available with ITK 6.0, but it is recommended to use 'CoordinateType' instead. The 'CoordRepType' aliases will be removed when 'ITK_FUTURE_LEGACY_REMOVE' is enabled. Similarly, 'InputCoordinateType', 'OutputCoordinateType', and 'ImagePointCoordinateType' replace 'InputCoordRepType', 'OutputCoordRepType', and 'ImagePointCoordRepType', respectively.
Replace add_library with itk_module_add_library macro for better integration with ITK module system. This provides automatic dependency linking, include directory setup, and consistent target naming.
Activates the newly ingested module in the configure-ci task and removes its Modules/Remote/*.remote.cmake stub.
Remove duplicate ENABLE_SHARED keyword and replace placeholder DESCRIPTION with a meaningful summary of the module's purpose.
Replace C-style casts that strip const with explicit const_cast on the result of GetInput() / dynamic_cast, and remove an unreachable return following itkExceptionMacro (which throws).
Replace the C-style cast that strips const from the dynamic_cast result with an explicit const_cast.
Applies to PhysicalCentralDifferenceImageFunction definitions, per ITK convention enforced by clang-tidy.
Replace the raw binary test fixtures (images, speed volumes, baselines)
with ExternalData content-link files and wrap their references in
DATA{...} so the blobs are fetched from ITKTestingData at build time
rather than committed into the ITK tree.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
AdvanceOneStep() only dispatched the fully-connected branch for spaceDimension 2 or 3; any other dimension fell through with bestValue unchanged, causing the optimizer to StopOptimization() at the start point with no diagnostic. Raise an exception so the unsupported case is surfaced rather than silently terminating path extraction.
The ingested upstream test/CMakeLists.txt used plain add_test() with
literal DATA{...} arguments. CMake's add_test() does not expand DATA{};
only itk_add_test() (via ExternalData_Add_Test) does, so the test
driver received the literal string "DATA{/.../Foo.mhd,Foo.zraw}" as a
filename and every data-driven test failed immediately. Switch the 18
data-driven tests to itk_add_test().
Synthetic_03_NeighborhoodIterate is marked WILL_FAIL: its multi-seed
baseline cannot be reproduced with either the upstream or the appended
front-insertion semantics; rebaselining is tracked as a post-ingest
follow-up so the remaining 17 tests can gate CI.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
hjmjohnson
force-pushed
the
ingest-MinimalPathExtraction
branch
from
9ac1142 to
55131ee
Compare
hjmjohnson
merged commit eb00442
into
InsightSoftwareConsortium:main
19 of 20 checks passed
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
14 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 ITKMinimalPathExtraction remote module into
Modules/Filtering/MinimalPathExtraction(group: Filtering). Source upstream:InsightSoftwareConsortium/ITKMinimalPathExtraction. Tracking issue: #6160.Ingest stats
Utilities/Maintenance/RemoteModuleIngest/INGESTION_STRATEGY.md)Modules/Filtering/MinimalPathExtraction/: 925f8dad2694a936aa65f0856285eb5fcb83a16d9eExternal-data fixtures
CID / .sha512 content-links present in this ingest:
(none -- module has no external-data fixtures)
If any fixtures resolve via ITKTestingData, that repo must contain the matching content-link before this PR can merge cleanly.
Follow-on commits (on top of the unrelated-histories merge)