Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Expose 3d point attribute p of SfmTrack in wrapper #537

Merged
merged 11 commits into from
Sep 23, 2020

Conversation

johnwlambert
Copy link
Contributor

@johnwlambert johnwlambert commented Sep 22, 2020

  • Grant SfmTrack 3d point access to wrapper
  • Update the list of updated python examples, to reflect available examples implemented already in C++

@@ -2847,6 +2847,7 @@ virtual class EssentialMatrixFactor : gtsam::NoiseModelFactor {

#include <gtsam/slam/dataset.h>
class SfmTrack {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d prefer the name point3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, so rename p to be point3? I think we will need to rename it in the following examples in this PR then since it's a breaking change:

  • examples/CreateSFMExampleData.cpp
  • timing/timeSFMBALnavTcam.cpp
  • timing/timeSFMBAL.cpp
  • examples/SFMExample_bal_COLAMD_METIS.cpp
  • examples/SFMExampleExpressions_bal.cpp
  • examples/SFMExample_bal.cpp

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, rename get3dPoint to point3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, @dellaert do you mind confirming that and I can quickly make the change? I guess “get*()” is not preferred in GTSAM?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, no get...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed the update, should be good to go now

@dellaert dellaert merged commit f6ef1d6 into borglab:develop Sep 23, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants