borglab / gtsam-project-python

Project template using GTSAM + python wrapping
BSD 2-Clause "Simplified" License
57 stars 15 forks source link

Autogenerate required gtsam import #12

Closed RunOrVeith closed 3 years ago

RunOrVeith commented 3 years ago

If you inherit a gtsam class such as gtsam::NoiseModelFactor2, you need to import gtsam before your own python module, otherwise it will crash due to something similar to this issue.

It took us quite some time to find what the issue is, because the error is not very clear.

import my_module 

Results in the following error. VelocityFactor is the custom factor that I implemented, it inherits from gtsam::NoiseModelFactor2 and is exposed in the header as

// Forward declarations as mandated in https://github.com/borglab/gtsam/blob/develop/gtsam/gtsam.i
virtual class gtsam::NonlinearFactor;
virtual class gtsam::NoiseModelFactor : gtsam::NonlinearFactor;

namespace my_module {
  #include <cpp/velocityFactor.h>  // Must be <>, not "". It will crash with a very confusing pyparsing error otherwise
  class VelocityFactor : gtsam::NoiseModelFactor
  {
    VelocityFactor();  # Just a placeholder
  };

}
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.8/site-packages/factor_graph_tracking-0.0.1-py3.8.egg/factor_graph_tracking/__init__.py", line 1, in <module>
    from .my_module import *
ImportError: generic_type: type "VelocityFactor" referenced unknown base type "gtsam::NoiseModelFactor"

It works if you do

import gtsam
import my_module

I wonder if the import could be autogenerated inside the genreated toplevel __init__.py file that at the moment only does from .my_module import *.

varunagrawal commented 3 years ago

Hi @RunOrVeith. I've experienced this before and I actually came up with a simple fix for this. My apologies for not adding the fix here sooner. I'll get on it.

varunagrawal commented 3 years ago

@RunOrVeith I've added the fix in the linked PR. It's a simple 1-line change that you can temporarily add in to get things going before we land the PR. :slightly_smiling_face: