Repository navigation
Fix MATLAB interface boundaries and enum value wrapping - #213
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved findings remain, and targeted regression coverage supports the generator fixes.
No open issues 🎉
What changed in this PR
Fixes MATLAB wrapper generation for interface-file boundaries, nested enum packages, and enum conversions using their declared owners.
Changes:
- Separates interface files with newlines to prevent trailing comments consuming declarations.
- Preserves namespace components and template aliases when resolving enum types.
- Adds regressions for file boundaries, enum ownership, and value conversions.
| File | Description |
|---|---|
| tests/test_matlab_wrapper.py | Adds boundary, namespace, alias, and enum conversion regressions. |
| tests/fixtures/matlab_enum_arguments.i | Adds cross-class enum coverage with later owner declarations. |
| tests/expected/matlab/enum_wrapper.cpp | Fully qualifies two expected C++ template enum types. |
| gtwrap/matlab_wrapper/wrapper.py | Fixes file separation, enum paths, registration, and conversions. |
| gtwrap/matlab_wrapper/mixins.py | Resolves declared enum owners while preserving template aliases. |
Review effort: Balanced
ProfFan
approved these changes
Oct 18, 2026
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
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.
Summary
Fix three MATLAB generator defects found while validating GTSAM's robust noise model wrappers:
//comment without a newline could swallow the next file's namespace or declarations.gtsam::noiseModel::mEstimator::Base::ReweightSchemebelongs in+gtsam/+noiseModel/+mEstimator/+Base, rather than+gtsamnoiseModelmEstimator/+Base.unwrap_shared_ptr<Enum>; passingBase.ReweightScheme.ScalartoAsymmetricCauchy.Createcrashed MATLAB after the path issue was corrected. Enum arguments now useunwrap_enum, and enum results use the owning MATLAB enum class.The enum lookup includes declarations appearing later in the input and preserves the MATLAB names of template instantiations. It is shared by arguments, properties, ordinary results, optional values, and pair results. The existing enum fixture changes only to fully qualify two C++ template enum types.
Validation
python -m pytest -o addopts='' tests: 137 passed on Python 3.12/macOS.AsymmetricCauchy.CreateandAsymmetricTukey.Createin MATLAB R2025b. The original crashing call now passes. Final generated C++ was verified byte-identical to the compiled source.GTSAM follow-up
GTSAM still needs to sync this subtree and adjust the separate explicit
std::shared_ptrdeclarations forRobust.Createand its accessors to the wrapper's supported shared-pointer notation. Those interface declaration changes belong in GTSAM.