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.

Sign up for GitHub

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

Jump to bottom

fix pybind11 high-level binds #1128

Merged
merged 2 commits into from
Nov 27, 2023
Merged

fix pybind11 high-level binds #1128

merged 2 commits into from
Nov 27, 2023

Conversation

andreaostuni
Copy link
Contributor

This PR solves the problem mentioned in #1127

@CLAassistant
Copy link

CLAassistant commented Nov 26, 2023 •
edited

CLA assistant check
All committers have signed the CLA.

traversaro
traversaro reviewed Nov 27, 2023
View reviewed changes
src/high-level/CMakeLists.txt
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/iDynTree/
PRIVATE_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/iDynTree/)
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/iDynTree
PRIVATE_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/iDynTree/impl)
Copy link
Member

Choose a reason for hiding this comment

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

Just a curiosity, did you modified this for consistency with the other components?

@traversaro
Copy link
Member

Thanks @andreaostuni ! Just a curiosity, why are you using the pybind11-bindings instead of the swig-based ones? Do you need a release "quick" with this changes?

@traversaro
Copy link
Member

traversaro commented Nov 27, 2023 •
edited

fyi @francesco-romano if you have any opinion.

@francesco-romano
Copy link
Collaborator

fyi @francesco-romano if you have any opinion.

Opinion on... what?

@traversaro
Copy link
Member

fyi @francesco-romano if you have any opinion.

Opinion on... what?

Sorry, on the PR!

@traversaro traversaro merged commit 0aafd95 into robotology:master Nov 27, 2023
18 checks passed
@andreaostuni
Copy link
Contributor Author

Thanks @andreaostuni ! Just a curiosity, why are you using the pybind11-bindings instead of the swig-based ones? Do you need a release "quick" with this changes?

I'm actually new in the world of bindings and I don't know the difference really well and since I had already had pybind I tried with it!

@traversaro
Copy link
Member

I see, thanks! Just a word of warning: most of Python code for iDynTree you may find around (or that LLMs models generate) is probably oriented toward the swig bindings of iDynTree.

@traversaro traversaro mentioned this pull request Dec 6, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@traversaro traversaro traversaro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@andreaostuni @CLAassistant @traversaro @francesco-romano