Skip to content

Wrap LabelSetMeasures in LabelOverlapMeasuresImageFilter#4221

Merged
hjmjohnson merged 4 commits intoInsightSoftwareConsortium:mainfrom
dzenanz:lsmLabelOverlap
May 4, 2026
Merged

Wrap LabelSetMeasures in LabelOverlapMeasuresImageFilter#4221
hjmjohnson merged 4 commits intoInsightSoftwareConsortium:mainfrom
dzenanz:lsmLabelOverlap

Conversation

@dzenanz
Copy link
Copy Markdown
Member

@dzenanz dzenanz commented Sep 19, 2023

This exposes the return value from itk::LabelOverlapMeasuresImageFilter::GetLabelSetMeasures() as a Python dictionary instead of <Swig Object of type 'std::unordered_map< unsigned char,itkLabelOverlapMeasuresImageFilterIUC3::LabelSetMeasures > *'>. This is needed for convenient use of the class from Python.

PR Checklist

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class area:Filtering Issues affecting the Filtering module labels Sep 19, 2023
@dzenanz dzenanz force-pushed the lsmLabelOverlap branch 2 times, most recently from 76a9429 to bf14387 Compare September 20, 2023 15:27
@github-actions github-actions Bot added type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct type:Data Changes to testing data labels Sep 27, 2023
@dzenanz
Copy link
Copy Markdown
Member Author

dzenanz commented Sep 28, 2023

Currently running into:

3024: Loading ITKImageStatistics... done
3024: Running itkLabelOverlapMeasuresImageFilterIUC3... Running itkLabelOverlapMeasuresImageFilterIUC3... done
3024: done
3024: Traceback (most recent call last):
3024:   File "C:\Dev\ITK-git\Modules\Filtering\ImageStatistics\wrapping\test\itkLabelOverlapMeasuresImageFilterTest.py", line 33, in <module>
3024:     for label, measure in lsm.items():
3024:                           ^^^^^^^^^
3024: AttributeError: 'SwigPyObject' object has no attribute 'items'
3024: swig/python detected a memory leak of type 'std::unordered_map< unsigned char,itkLabelOverlapLabelSetMeasures,std::hash< unsigned char >,std::equal_to< unsigned char >,std::allocator< std::pair< unsigned char const,itkLabelOverlapLabelSetMeasures > > > *', no destructor found.
3024: itkTestDriver: Process exited with return value: 1

@thewtex
Copy link
Copy Markdown
Member

thewtex commented Sep 29, 2023

What is dir(lsm)?

@dzenanz
Copy link
Copy Markdown
Member Author

dzenanz commented Sep 29, 2023

dir(lsm) is ['__class__', '__delattr__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__getstate__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__int__', '__le__', '__lt__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', 'acquire', 'append', 'disown', 'next', 'own']

@dzenanz dzenanz added this to the ITK 5.4.0 milestone Oct 12, 2023
@thewtex
Copy link
Copy Markdown
Member

thewtex commented May 2, 2024

@dzenanz could you please rebase and push? I am wondering if recent SWIG updates have helped.

@dzenanz dzenanz force-pushed the lsmLabelOverlap branch from 6962a00 to 828b59a Compare May 2, 2024 20:48
@thewtex thewtex modified the milestones: ITK 5.4.0, ITK 5.4.1 May 16, 2024
@thewtex thewtex modified the milestones: ITK 5.4.2, ITK 5.4.3 Jan 20, 2025
@thewtex thewtex modified the milestones: ITK 5.4.3, ITK 5.4.4 Mar 19, 2025
@thewtex thewtex modified the milestones: ITK 5.4.4, ITK 5.4.5 Jun 9, 2025
@hjmjohnson
Copy link
Copy Markdown
Member

Rebased onto current upstream/main (was 3894 commits behind). Both commits cherry-picked cleanly with only auto-merges in itkLabelOverlapMeasuresImageFilter.h and Wrapping/Generators/Python/PyBase/pyBase.i — no manual conflict resolution needed. CI will re-run on the new HEAD; the SWIG-update question @thewtex raised in May 2024 should now answer itself.

hjmjohnson added a commit to dzenanz/ITK that referenced this pull request May 2, 2026
end-of-file-fixer pre-commit hook flags a trailing empty line on this
upstream-tracked file.  Same one-character fix as PR InsightSoftwareConsortium#6032; included
here so PR InsightSoftwareConsortium#4221's pre-commit CI can pass without waiting on that PR
to merge first.
@github-actions github-actions Bot added the area:Documentation Issues affecting the Documentation module label May 2, 2026
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 2, 2026
Same upstream-inherited end-of-file-fixer hit blocking PR InsightSoftwareConsortium#6032 / InsightSoftwareConsortium#4221.
One-character fix included here so this PR's pre-commit CI can pass.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 2, 2026
`Documentation/docs/releases/5.4.6.md` carries a trailing empty line
that the `end-of-file-fixer` pre-commit hook flags every time a PR
rebases onto current `main`.  Affected at least PRs InsightSoftwareConsortium#6032, InsightSoftwareConsortium#4221, and
InsightSoftwareConsortium#6186, where each had to carry an identical one-character fix.  Apply
once on `main` to stop the spurious `pre-commit` failures.
@github-actions github-actions Bot removed the area:Documentation Issues affecting the Documentation module label May 2, 2026
@hjmjohnson
Copy link
Copy Markdown
Member

Pushed two commits on top of lsmLabelOverlap that take a different angle on the Python-side test failure.

Diagnosis (verified locally, macOS Python wrapping build): SWIG silently drops %template std::unordered_map<X, itk::LabelOverlapLabelSetMeasures> in the Instantiations.i because the value type is %import-ed (not %include-d) into the consuming submodule. Count of hashmap in the SWIG-generated ITKImageStatisticsPython.cpp was 0 across every namespace-qualifier and ordering variant tried. Confirmed both inside Modules/Filtering/ImageStatistics/wrapping/ and after relocating the %template declarations to pyBase.i — the second attempt failed for layering reasons (pyBase.i is foundation and can't see leaf-module headers).

Pivot to a wrapping-friendly C++ surface: added two small accessors that don't rely on std::unordered_map getting a Python-dict wrapper. The existing MapType GetLabelSetMeasures() is preserved unchanged for C++ consumers.

std::vector<LabelType>      GetLabels() const;
LabelOverlapLabelSetMeasures GetMeasureForLabel(LabelType) const;

Plus six Get* getters on the struct itself (igenerator doesn't auto-wrap public data members). Both work natively across the per-submodule SWIG isolation because std::vector<unsigned char> is already wrapped globally in pyBase.i and the struct is wrapped via the existing itk_wrap_simple_class directive.

Test now passes locally against the existing fixture:

Found 14 labels
Label: 1, i: 2246, u: 120691
Label: 2, i: 497,  u: 22835
…
Label: 13, i: 0,   u: 387126
1/1 Test #211: LabelOverlapMeasuresImageFilterTest ...   Passed    1.75 sec
100% tests passed, 0 tests failed out of 1
Commits (2, on top of your existing 2)
Commit Files
3 ENH: Add Python-friendly LabelSetMeasures accessors itkLabelOverlapMeasuresImageFilter.h, itkLabelOverlapLabelSetMeasures.h
4 COMP: Drop unused std::unordered_map SWIG infrastructure wrapping/CMakeLists.txt (drop WRAPPER_SWIG_LIBRARY_FILES), delete itkLabelOverlapLabelSetMeasuresInstantiations.i, revert pyBase.i %include <std_unordered_map.i>, update Python test

Both commits carry Co-Authored-By: dzenanz to preserve your authorship.

What I tried before pivoting (so this isn't a black box)
Variant Result
Original (itk::LabelOverlapLabelSetMeasures qualified, in Instantiations.i) %template silently dropped — 0 hashmap classes generated
Drop the itk:: namespace qualifier SWIG generates templates but C++ can't compile (the flat name only exists as a typedef in the struct's submodule)
Add using itkLabelOverlapLabelSetMeasures = itk::LabelOverlapLabelSetMeasures; to Instantiations.i's %{…%} Templates still dropped
Move %template to pyBase.i with %import "itkLabelOverlapLabelSetMeasures.h" SWIG fails: header not on PyBase's include path
Forward-declare in pyBase.i (namespace itk { struct LabelOverlapLabelSetMeasures; }) SWIG generates code; C++ compile fails for layering reasons

Root cause is architectural: ITK's wrapping pipeline gives each submodule its own SWIG run with %import-only type info from siblings, and SWIG's %template std::unordered_map<X, Y> requires Y to be %include-d (full type definition) — not just declared. Fixing this cleanly would be a multi-day igenerator.py change with broad blast radius.

CI will re-run on 2e882122e2. If anything in this approach doesn't sit right with you, happy to iterate — the commits are surgical and easy to drop / amend.

@hjmjohnson
Copy link
Copy Markdown
Member

Re-triggering ARMBUILD-x86_64-rosetta — itkImageAlgorithmCopyTest2 (3.14 float CheckBuffer) failed in isolation on this runner; same test passed on #6185/#6188/#6189 against the same runner pool, and CDash shows 0 test failures (Linux 3486/3486, Darwin 309/309, Linux.Python 176/176). Known x86_64-rosetta float-precision flake, unrelated to this PR's diff.

@hjmjohnson hjmjohnson marked this pull request as ready for review May 3, 2026 17:29
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 3, 2026

Greptile Summary

This PR extracts LabelOverlapMeasuresImageFilter::LabelSetMeasures into a standalone LabelOverlapLabelSetMeasures struct with explicit Get* accessors, and adds GetLabels() + GetMeasureForLabel() methods to the filter, together with a new Python wrapping and test. This allows Python consumers to iterate label measures without hitting SWIG's inability to wrap std::unordered_map across submodule boundaries. The backward-compatibility typedef is retained under #ifndef ITK_FUTURE_LEGACY_REMOVE.

Confidence Score: 4/5

Safe to merge; only P2 style findings (misleading comment, extra blank line, no value assertions in test).

All findings are P2 — no logic errors, no API breaks, and the backward-compat alias is correctly guarded. Score reflects clean design with minor polish items.

itkLabelOverlapMeasuresImageFilter.h — misleading doc comment on the legacy typedef and an extra blank line.

Important Files Changed

Filename Overview
Modules/Filtering/ImageStatistics/include/itkLabelOverlapLabelSetMeasures.h New standalone header extracting LabelOverlapLabelSetMeasures struct with explicit Get* accessors for Python wrapping; well-structured and self-contained.
Modules/Filtering/ImageStatistics/include/itkLabelOverlapMeasuresImageFilter.h Adds GetLabels() + GetMeasureForLabel() accessors for Python, moves struct to standalone header; copy-pasted misleading doc comment on legacy typedef and a double blank line.
Modules/Filtering/ImageStatistics/wrapping/itkLabelOverlapLabelSetMeasures.wrap Minimal one-liner wrap file using itk_wrap_simple_class; correct approach for a plain struct.
Modules/Filtering/ImageStatistics/wrapping/CMakeLists.txt Sets WRAPPER_SUBMODULE_ORDER to ensure LabelOverlapLabelSetMeasures is wrapped before the filter that uses it; correct ordering.
Modules/Filtering/ImageStatistics/wrapping/test/itkLabelOverlapMeasuresImageFilterTest.py Python smoke test exercising GetLabels() + GetMeasureForLabel() and Get* accessors; no value assertions so numerical regressions would go undetected.
Modules/Filtering/ImageStatistics/test/Input/DzZ_Seeds.seg.nrrd.sha512 Valid 128-character SHA-512 hash for new test input file.
Modules/Filtering/ImageStatistics/test/Input/DzZ_T1.seg.nrrd.sha512 Valid 128-character SHA-512 hash for new test input file.
Modules/Filtering/ImageStatistics/wrapping/test/CMakeLists.txt New wrapping test CMakeLists with proper guards for 3D uchar wrapping; correct structure.

Sequence Diagram

sequenceDiagram
    participant Py as Python caller
    participant F as LabelOverlapMeasuresImageFilter
    participant M as std::unordered_map (m_LabelSetMeasures)
    participant S as LabelOverlapLabelSetMeasures

    Py->>F: UpdateLargestPossibleRegion()
    F->>M: populate per-label entries
    Py->>F: GetLabels()
    F->>M: iterate keys
    F-->>Py: std::vector<LabelType>
    loop for each label
        Py->>F: GetMeasureForLabel(label)
        F->>M: find(label)
        M-->>F: iterator → LabelOverlapLabelSetMeasures
        F-->>Py: LabelOverlapLabelSetMeasures (by value)
        Py->>S: GetIntersection() / GetUnion() / …
        S-->>Py: SizeValueType
    end
Loading

Reviews (1): Last reviewed commit: "COMP: Drop unused std::unordered_map SWI..." | Re-trigger Greptile

Comment thread Modules/Filtering/ImageStatistics/include/itkLabelOverlapMeasuresImageFilter.h Outdated
Copy link
Copy Markdown
Member Author

@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.

Looks good. I can't approve my own PR.

dzenanz and others added 4 commits May 4, 2026 15:39
This exposes the return value from
itk::LabelOverlapMeasuresImageFilter::GetLabelSetMeasures()
as a Python dictionary instead of
<Swig Object of type 'std::unordered_map< unsigned char,itkLabelOverlapMeasuresImageFilterIUC3::LabelSetMeasures > *'>
This is needed for convenient use of the class from Python.
The class has been un-nested to make wrapping easier/possible.
Expose the per-label overlap measures to Python via two paired
accessors on itk::LabelOverlapMeasuresImageFilter:

  std::vector<LabelType> GetLabels() const
  LabelOverlapLabelSetMeasures GetMeasureForLabel(LabelType) const

Plus six explicit `Get*()` getters on itk::LabelOverlapLabelSetMeasures
(GetSource, GetTarget, GetUnion, GetIntersection, GetSourceComplement,
GetTargetComplement) so Python can read the per-label fields.

The existing C++ API (`MapType GetLabelSetMeasures()` returning
`std::unordered_map<LabelType, LabelOverlapLabelSetMeasures>`) is
preserved unchanged for performant in-process lookup; the new
accessors are additive.

Why two accessors instead of wrapping the unordered_map as a Python
dict: SWIG's %template instantiation of std::unordered_map<X,Y> does
not materialize a dict-like wrapper class when Y (the value type) is
%import-ed (rather than %include-d) into the consuming submodule.
ITK's per-submodule wrapping pipeline isolates each class as its own
SWIG run with %import-only type info from siblings, so the
%template directives in itkLabelOverlapLabelSetMeasuresInstantiations.i
were silently dropped (count of `hashmap` in the SWIG-generated .cpp
was 0).  Falling back to a vector-of-keys + per-key lookup avoids the
%template materialization issue entirely; std::vector<LabelType> is
already wrapped globally in pyBase.i for every primitive label type.

Co-Authored-By: dzenanz <dzenanz@gmail.com>
The original PR added wrapping infrastructure to expose
`std::unordered_map<LabelType, LabelOverlapLabelSetMeasures>` as a
Python dict.  In practice the %template instantiations in
itkLabelOverlapLabelSetMeasuresInstantiations.i were silently dropped
by SWIG: the value type LabelOverlapLabelSetMeasures is %import-ed
(not %include-d) into the LabelOverlapMeasuresImageFilter submodule,
so SWIG had only a forward declaration and could not materialize the
hashmap wrapper class.  Confirmed locally on macOS: count of `hashmap`
in the SWIG-generated ITKImageStatisticsPython.cpp was 0 across every
namespace-qualifier and ordering variant tried.

Drop the unused infrastructure so the PR ships only the working
C++/Python surface introduced in the previous commit:

  * Remove WRAPPER_SWIG_LIBRARY_FILES wiring from
    Modules/Filtering/ImageStatistics/wrapping/CMakeLists.txt.
  * Remove itkLabelOverlapLabelSetMeasuresInstantiations.i (no
    longer referenced).
  * Revert the `%include <std_unordered_map.i>` line added to
    Wrapping/Generators/Python/PyBase/pyBase.i (unused after the
    above; pyBase.i is foundation-level shared infrastructure and
    should not carry a header it doesn't consume).

Update itkLabelOverlapMeasuresImageFilterTest.py to use the new
GetLabels()/GetMeasureForLabel() accessors and the explicit getters
(GetIntersection, GetUnion) on the per-label struct.  Local test
output (200 voxel-label fixture, 14 labels detected, intersection
and union counts match expected per-label values):

  Found 14 labels
  Label: 0, ...  Label: 1, i: 2246, u: 120691  ...  Label: 13, i: 0, u: 387126
  100% tests passed, 0 tests failed out of 1

Co-Authored-By: dzenanz <dzenanz@gmail.com>
@hjmjohnson hjmjohnson merged commit 519bfa0 into InsightSoftwareConsortium:main May 4, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module area:Python wrapping Python bindings for a class type:Data Changes to testing data type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants