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

Starting point for pybind11 bindings #351

Closed

Conversation

lamiller0
Copy link
Contributor

Very very early WIP to start building out pybind11 versions of CMake

Bring over a bunch of the constants, but declare them as pybind11 enums.

I'm not sure yet how best to expose some of the stuff PyAlembic will eventually need, and I needed to put PyImathVec.cpp in MODSOURCE as the register symbols were not being found in LIBSOURCE.

Bring over a bunch of the constants, but declare them as pybind11 enums.

I'm not sure yet how best to expose some of the stuff PyAlembic will
eventually need, and I needed to put PyImathVec.cpp in MODSOURCE as the
register symbols were not being found in LIBSOURCE.
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 6, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: lamiller0 / name: Lucas Miller (fff3509)

@lamiller0 lamiller0 marked this pull request as draft October 7, 2023 00:00
@cary-ilm
Copy link
Member

Thanks for this! The ASWF projects require signed commits, or DCO (Developer Certificate of Origin), which basically means creating commits with the -s option, which adds a line certifying that the change originated with you. You should be able to add this via:

% git commit --amend -s
% git push --force

@cary-ilm
Copy link
Member

We started a side email discussion about the feature branch, and @lgritz advised against a feature branch, which makes sense. How about we discard the pybind11 branch and merge this into main, but create an entirely new directory parallel to src/python called src/pybind11 so that we don't intermingle the new pybind11 with the existing boost implementation. Then a single top-level CMake option to add ``src/pybind11` subdirectory. That should make it easy to support development on the new stuff while still maintaining/releasing the current stuff.

I'm open to other suggestions, but I'd prefer to not introduce a pybind11 dependency until we're ready.

@cary-ilm
Copy link
Member

Also, we'll need the CI to install pybind11, since as it is now, the build is not finding it so the checks are ignoring the new library. That should presumably be done in the docker container.

@cary-ilm
Copy link
Member

CMake is failing for me on ImathTestC:


CMake Error at src/python/PyImathTest/CMakeLists.txt:23 (target_link_libraries):
  Target "PyImathTestC" links to:

    Imath::PyImath_Python3_10

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

This may be a problem with how I have boost installed on my machine, but just confirming, this works for you?

@lamiller0
Copy link
Contributor Author

My initial intention of putting it in src/python was because I was hoping to be able to easily use PyImathTest, but looking a bit closer at it I realize now that its not python unit tests. I dont mind moving it into an adjacent pybind11 and making the other changes you requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants