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

Python binding for Vec questions #355

Open
lamiller0 opened this issue Oct 14, 2023 · 3 comments
Open

Python binding for Vec questions #355

lamiller0 opened this issue Oct 14, 2023 · 3 comments

Comments

@lamiller0
Copy link
Contributor

Here are some questions that came up while doing the pybind11 bindings and comparing it with the boost python bindings.

Vec4 doesnt have any cross product operators or functions in it. The C++ lacks it as well so I'm guessing its because its impossible?

Vec2 is missing %= operator but has the other cross product operators on the C++ side, is this an oversight that I can just add in on the C++ side?

closestVertex in C++ has it's args like: V0, V1, V2, P but in boost python its: P, V0, V1, V2 is this on purpose?
https://github.com/lamiller0/Imath/blob/main/src/python/PyImath/PyImathVec3Impl.h#L237

project in C++ takes v, v0 but the boost python binding flips it (v0, v)
https://github.com/lamiller0/Imath/blob/main/src/python/PyImath/PyImathVec3Impl.h#L261

@lgritz
Copy link
Contributor

lgritz commented Oct 14, 2023

Cross product only makes mathematical sense in 3 dimensions.

@lgritz
Copy link
Contributor

lgritz commented Oct 14, 2023

As for the final two things you mention... oof. Well that kinda sucks. We would like the C++ and Python interfaces to match, of course. We would also like to not break any interfaces that have existed a long time.

I would guess that neither of those are on purpose, but both probably mistakes from the early days that were either never noticed, or whoever walked this path before us also threw up their hands and decided to leave it broken for the sake of not breaking any python code that was already using the jumbled argument order.

@meshula
Copy link
Contributor

meshula commented Oct 14, 2023

Could we deprecate the existing closestVertex and project in Python (leaving them in place) and provide new ones with new names and matching argument order (closeVertexToPoint, projectVectorOnVector? Seems like the right time to do something like that.

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

No branches or pull requests

3 participants