borglab/wrap

Separate Pybind modules for inner namespaces

Opened this issue · 6 comments

Feature

Currently, all namespaces under the top level gtsam (e.g. symbol_shorthand) are added as pybind submodules. This requires the python import to look like

from gtsam.gtsam.symbol_shorthand import X

aka a gtsam module inside a gtsam module.

We can get rid of the inner gtsam module by configuring the pybind wrapper to create separate modules for all the inner namespaces, allowing us to do

from gtsam.symbol_shorthand import X

which is more intuitive and neater, especially since import gtsam.symbol_shorthand works.

Motivation

Outlined above.

Pitch

Update the Pybind wrapper to have separate modules for each inner namespace.
Currently there are 5:

  • noiseModel
  • mEstimator
  • symbol_shorthand
  • imuBias
  • utilities

Alternatives

N/A

Additional context

@ProfFan should be able to provide details on whether this is doable without breaking other stuff since he's the lead on the Pybind wrapper. :-)

It's just time consuming 🤷 I feel it's more like a cosmetic issue (and I feel that it may be possible to solve with a trick)

[spoiler]

add a symbol_shorthands subfolder, put a __init__.py there, and add delegates.

I do agree with @varunagrawal there is an issue here. @ProfFan cosmetics matters for our users.
Also, "adding delegates" may be something that adds maintenance issues?
Typing is not the bottleneck here so I'd like to avoid tricks if possible.

@dellaert It's because we are currently having this one massive header and it's not trivial to escape this design pattern, as the wrap script is designed for this. Maybe in the future we can split the headers so this won't be an issue, but for now, I don't see an easy way to get this working.

So I actually did quite a bit of research on this today. I'd be glad to put in a PR for this once I'm done with my summer commitments.

Could you write up the solution in an issue? Maybe with links?

Or, this issue, of course :-)