Skip to content

COMP: Update CI workflows for remote module#40

Merged
hjmjohnson merged 4 commits intomainfrom
update-to-new-macros
Mar 9, 2025
Merged

COMP: Update CI workflows for remote module#40
hjmjohnson merged 4 commits intomainfrom
update-to-new-macros

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

  • STYLE: Add itkVirtualGetNameOfClassMacro + itkOverrideGetNameOfClassMacro
  • STYLE: Update to match clang-format-19 from ITK
  • ENH: Update to support the clang-format-linter CI
  • COMP: Update workflow package dependancy versions.
  • COMP: Select latest remote build branch.

@dyollb
Copy link
Copy Markdown

dyollb commented Feb 3, 2025

@hjmjohnson thanks for all your efforts to get this through! I see it has been quite an odyssey. I need this module too, but a nice workaround (for me) would be to enable this module in SimpleITK wheels on pypi. @blowekamp would this be of any interest?

@blowekamp
Copy link
Copy Markdown
Member

@hjmjohnson thanks for all your efforts to get this through! I see it has been quite an odyssey. I need this module too, but a nice workaround (for me) would be to enable this module in SimpleITK wheels on pypi. @blowekamp would this be of any interest?

@dyollb These filters are already specified in SimpleITK:
https://github.com/SimpleITK/SimpleITK/blob/master/Code/BasicFilters/json/LabelSetDilateImageFilter.json
https://github.com/SimpleITK/SimpleITK/blob/master/Code/BasicFilters/json/LabelSetErodeImageFilter.json

But as a remote module they are turned off by default. If you compile SimpleITK yourself it can be enabled:
https://simpleitk.readthedocs.io/en/master/building.html#advanced-build-options

The SimpleITK pypi binaries can be large and the the compilation and linking cumbersome. However we can evaluate the impact of these filter. Can you make a PR to simpleITK which enables this module by default?
ref: https://github.com/SimpleITK/SimpleITK/blob/master/SuperBuild/External_ITK.cmake#L27

@hjmjohnson hjmjohnson force-pushed the update-to-new-macros branch 7 times, most recently from 5791d63 to 0fd381e Compare February 4, 2025 22:26
@jhlegarreta
Copy link
Copy Markdown
Member

Thanks for doing this Hans.

In case it is of any help, the errors on the main branch:

itktiff-5.3.lib(tif_luv.c.obj) : error LNK2005: __ucrt_int_to_float already defined in itktiff-5.3.lib(tif_aux.c.obj)
itktiff-5.3.lib(tif_color.c.obj) : error LNK2005: __ucrt_int_to_float already defined in itktiff-5.3.lib(tif_aux.c.obj)

https://github.com/InsightSoftwareConsortium/ITKLabelErodeDilate/actions/runs/13101850892/job/36551067110#step:8:3035

Are already taken care of when using the v5.4.0 branch of the remote action:
InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction@main...v5.4.0

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Mar 8, 2025

There are conflicts, can you rebase?

…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 699b66cb04d410e555656828e8892107add38ccb, Will
Schroeder, June 27, 2001:
https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
@hjmjohnson hjmjohnson force-pushed the update-to-new-macros branch from 6c43191 to 87389e9 Compare March 9, 2025 12:51
@hjmjohnson hjmjohnson merged commit 22d8846 into main Mar 9, 2025
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants