Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Clean up the __init__ #893

Merged
merged 3 commits into from
Oct 18, 2021
Merged

Clean up the __init__ #893

merged 3 commits into from
Oct 18, 2021

Conversation

varunagrawal
Copy link
Collaborator

Simple PR to do some wrapper clean up.

@varunagrawal varunagrawal added the python Related to python wrapper label Oct 18, 2021
@varunagrawal varunagrawal self-assigned this Oct 18, 2021
@varunagrawal varunagrawal added the quick-review Quick and easy PR to review label Oct 18, 2021
import sys

from gtsam import gtsam, utils
from gtsam.gtsam import *
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for the PR @varunagrawal. While we're updating this, we probably want to remove this wildcard import too.

Wildcard imports discussed in PEP8

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is needed, and I think is generally OK for pybind11 modules.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup this is needed for proper functioning of the module.


pybind_wrap(gtsam_py # target
pybind_wrap(${GTSAM_TARGET} # target
Copy link
Collaborator

Choose a reason for hiding this comment

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

GTSAM_PYTHON_TARGET

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah great point!

@@ -122,7 +124,7 @@ if(GTSAM_UNSTABLE_BUILD_PYTHON)
gtsam::CameraSetCal3Fisheye
gtsam::KeyPairDoubleMap)

pybind_wrap(gtsam_unstable_py # target
pybind_wrap(${GTSAM_UNSTABLE_TARGET} # target
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

import sys

from gtsam import gtsam, utils
from gtsam.gtsam import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is needed, and I think is generally OK for pybind11 modules.

@varunagrawal varunagrawal merged commit 9e032e4 into develop Oct 18, 2021
@varunagrawal varunagrawal deleted the feature/clean-import branch October 18, 2021 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Related to python wrapper quick-review Quick and easy PR to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants