python: wrapper fixes for DefaultKeyFormatter stubs and AttitudeFactor noise model

[DLuminary]

Two unrelated defects in the Python wrapper, one commit each.

1. gtsam.DefaultKeyFormatter is the wrapper's only module-level variable. wrap
   emits it as m_.attr(...) = gtsam::DefaultKeyFormatter, and pybind11 gives that
   std::function value no __module__ or __name__, so pybind11-stubgen cannot
   resolve it and drops it from the stubs; type checkers then reject a symbol that
   works at runtime and that gtsam's own Python files use (hidden today by
   --ignore-all-errors on the stub targets). Fix: re-register it once in gtsam.tpl,
   after the generated bindings, as a real function. Behaviour is unchanged.
   Declaring it as a function in gtsam.i instead would also make the MATLAB wrapper
   emit a new binding (verified by running it on both declarations), so the fix
   stays pybind-only. The template block runs exactly once and pybind11 replaces
   the attribute rather than chaining an overload; verified on a minimal module,
   where the symbol then appears in the stub. Adds test_DefaultKeyFormatter.py.
   Fixed by Bind std::function variables as named Python functions wrap#209 (when merged and squashed).

2. AttitudeFactor's C++ constructor takes a SharedNoiseModel, but navigation.i
   declared it as noiseModel::Diagonal*, rejecting Robust and full-covariance
   Gaussian models from Python. Declare it as Base*, as the sibling navigation
   factors do, for all VALUE instantiations. An audit of every interface file
   found no other declaration narrower than its C++ signature. Adds a regression
   test with a robust and a full-covariance model.

[Copilot]

🟡 Changes recommended

The new stub regression test can silently skip under common test flows (stubs not generated), weakening the intended guard against regressions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes two Python-wrapper defects: ensuring gtsam.DefaultKeyFormatter is visible to type checkers by making it a real pybind11 function (so stub generation can resolve it), and widening the wrapped AttitudeFactor constructor signature to accept any noiseModel.Base (matching the C++ API).

Changes:

• Re-register DefaultKeyFormatter in the pybind11 template as a proper function to preserve __module__/__name__ metadata for stub generation.
• Update AttitudeFactor wrapper constructors to accept noiseModel::Base* instead of noiseModel::Diagonal*.
• Add Python regression tests covering both wrapper fixes.

File summaries

File	Description
python/gtsam/tests/test_DefaultKeyFormatter.py	Adds regression coverage for runtime behavior and stub visibility of DefaultKeyFormatter.
python/gtsam/tests/test_AttitudeFactor.py	Adds a regression test that constructs AttitudeFactor with robust and full-covariance noise models.
python/gtsam/gtsam.tpl	Re-defines DefaultKeyFormatter via m_.def(...) to make it stubgen-friendly.
gtsam/navigation/navigation.i	Widens the wrapped AttitudeFactor constructor noise-model type to noiseModel::Base*.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dellaert]

Thanks !
See copilot review, and there is a a CI failure.

[ProfFan]

Thanks for tracking down the DefaultKeyFormatter stub issue. The diagnosis and m_.def(...) fix look correct. I think the general fix belongs in wrap, since other std::function variables can encounter the same missing metadata.

I've opened borglab/wrap#209 to handle this in the generator. It uses C++ overload resolution to bind std::function variables, including aliases such as KeyFormatter, as named Python functions while preserving ordinary variable bindings.

Once that change is merged and synced into GTSAM, it should address this issue without the extra binding in gtsam.tpl. The AttitudeFactor noise-model fix is independent and can proceed separately. Thanks again for identifying both issues!

[DLuminary]

Agreed, borglab/wrap#209 is the right layer — the overload trick resolves the alias in C++ so the generator needs no special-casing. Dropping the gtsam.tpl commit. The AttitudeFactor commit stands on its own.

Note that the former CI failure was due to:
sccache: error: Connection to server timed out: Os { code: 111, kind: ConnectionRefused, message: "Connection refused" } ninja: build stopped: subcommand failed. Error: Process completed with exit code 2.