borglab / gtsam-project-python

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

Unable To Get Example Working #8

Closed mawallace closed 4 years ago

mawallace commented 4 years ago

I'm having a hard time getting the example working as is.

The installation instructions are a little hard to follow, since the directory names seem to have been changed.

I tried following the instructions starting from the python directory, and I'm able to python setup.py install the module. However, I'm then getting

>>> import gtsam_example
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/.../gtsam-project-python/python/gtsam_example/__init__.py", line 1, in <module>
    from .gtsam_example import *
ModuleNotFoundError: No module named 'gtsam_example.gtsam_example'

when I try to import.

I would appreciate any guidance or clarification to the documentation. If someone is able to help me get this up and running, I'm happy to help update documentation for future users.

dellaert commented 4 years ago

We are aware of the issues, I feel it’s too complicated as well. I’m having a meeting with @varunagrawal soon on how to fix it. In the meantime he could perhaps fix the docs.

dellaert commented 4 years ago

It might already be fixed in the pending PR

varunagrawal commented 4 years ago

I am aware of some of the issues with the previous wrapper. The new PR simplifies a lot of the CMake and other things so it should work better.

I've also expanded on the tutorial some more, so we should definitely land the new PR.

mawallace commented 4 years ago

I just checked out the PR and was able to follow the compilation and installation instructions in TUTORIAL.md (verified by importing gtsam_example and interacting with a Greeting object in Python).

I didn't pay much attention to the actual changes in the PR, but from a user's perspective it's much easier to get the example up and running, and would definitely resolve this issue. Thanks!

I noticed that installation instructions in README.md would still be outdated. I'm happy to update that once the PR is in.

varunagrawal commented 4 years ago

Yay! I'm glad the updates work well. Like @dellaert mentioned, we are planning some more updates so I'll be sure to update the README too.

Thanks @mawallace.

varunagrawal commented 4 years ago

Hi @mawallace. PR #6 was merged in so I'm going ahead and closing this issue. I have create #9 to track the remaining changes and updates.