Skip to content

ENH: Add TractographyTRX Python wrappers#5951

Merged
dzenanz merged 1 commit intoInsightSoftwareConsortium:mainfrom
PennLINC:tractography-trx-pywrappers
Mar 16, 2026
Merged

ENH: Add TractographyTRX Python wrappers#5951
dzenanz merged 1 commit intoInsightSoftwareConsortium:mainfrom
PennLINC:tractography-trx-pywrappers

Conversation

@mattcieslak
Copy link
Contributor

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

This PR updates the TractographyTRX remote module to have python wrappers.
It also changes the Eigen config so that the remote module can be built as part of the ANTs superbuild.

@github-actions github-actions bot added type:Enhancement Improvement of existing methods or implementation area:Remotes Issues affecting the Remote module labels Mar 16, 2026
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WRAPPER_SUBMODULE_ORDER should only be set if the order of wrapping matters, e.g. one class's wrapping depends on the other. Otherwise you don't need to list them, itk_auto_load_and_end_wrap_submodules() will automatically pick up all the .wrap files. I think that if you don't manually order them, the build system has the opportunity to process them in parallel. But this is an optimization, this PR is good as-is. It builds and passes unit tests locally for me.

@dzenanz dzenanz merged commit d338c1f into InsightSoftwareConsortium:main Mar 16, 2026
6 checks passed
@mattcieslak
Copy link
Contributor Author

Thanks! I added the ordering as a trick to get it to compile, there was an issue having CompositeTransform/AffineTransform type as an argument for TransformInPlace.

@blowekamp
Copy link
Member

And update and refactor to ITK's Eigen was merged on Friday after noon: #5831

What this update built to work with that? The update should simplify usage of Eigen3 from ITK.

I'd recommend linking against "ITK::ITKEigen3Module" in your remote module, and library. This way the using the ITK package will provide the interface requirements and remove the need to additional find_packages for the Eigen library.

@dzenanz
Copy link
Member

dzenanz commented Mar 16, 2026

I started building local ITK with latest version to test this updated remote.

@dzenanz
Copy link
Member

dzenanz commented Mar 16, 2026

The build fails with the current main:

CMake Error at C:/Misc/ITKTractographyTRX-vs22/_deps/trx_cpp-src/CMakeLists.txt:121 (TARGET_LINK_LIBRARIES):
  Target "trx" links to:

    Eigen3::Eigen

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Remotes Issues affecting the Remote module type:Enhancement Improvement of existing methods or implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants