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
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…elOverlap

Wrap LabelSetMeasures in LabelOverlapMeasuresImageFilter
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