-
Notifications
You must be signed in to change notification settings - Fork 767
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
Compile gtsam python for windows #1685
Compile gtsam python for windows #1685
Conversation
edfe7ca
to
c9651e2
Compare
I will disable the running of tests for windows. that you have a clean CI. |
c9651e2
to
72f6f4e
Compare
Thank you! Could you post which tests fail here? |
I am post here the errors, also I am put a link for the run:
|
It seems to be an RTTI issue, which actually makes the Python wrappers very cumbersome to use.... |
I am not sure what are you suggesting. |
I am not sure what do you mean by RTTI issue. That was thrown from here: Maybe is msvc bug? On windows it choose the general case. Also for linux:
|
Yes, that is an RTTI issue, because |
I suspect it before the dynamic cast. It from template specialization. |
I fixed this problem before, don't know why it's coming up again. Culprit is likely |
@ProfFan I added a workaround for windows template specialization.
@varunagrawal @ProfFan Edit: |
3d1eb38
to
3c2ff14
Compare
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contributions! I'll leave the rest of review to @varunagrawal who is better suited as Windows reviewer. Also CC @mattking-smith @jlblancoc who may be more familiar with Windows.
Except the name changes, all the changes here are must and necessary to solve the problem of compiling windows gtsam python. |
369eff9
to
2a6d4ff
Compare
95cb7aa
to
d1ab94f
Compare
This branch is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asking for one small change and then I can approve this PR.
Compile gtsam python for windows
I manage to compile and run python tests on windows.
Some of the python tests are fails on windows (16 of 243).
Please take a look on my PR.
@dellaert @varunagrawal @ProfFan
Waiting for merge: