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

Non-void methods using call by reference #89

Open
voneiden opened this issue Jul 2, 2022 · 4 comments
Open

Non-void methods using call by reference #89

voneiden opened this issue Jul 2, 2022 · 4 comments

Comments

@voneiden
Copy link

voneiden commented Jul 2, 2022

There are quite a few methods in OCP that use call by reference but return a value instead of void. The current implementation of pywrap classifies functions as call by reference only when the return is void.

One example in OCP is the GeomLib_Tool.Parameter_s function - the computation result is stored in the 4th argument and the function returns a Standard_Boolean indicating whether the function succeeded in finding a value. However because the function does not return void, it's not classified as call by reference and therefore the actual computed value is not accessible from callers side.

I've opened an issue to CadQuery/pywrap#45 and a draft PR CadQuery/pywrap#46 demonstrating a possible solution (only for functions returning Standard_Boolean). This solution adds the boolean return value to the tuple returned by the wrapper.

A downside in implementing a change like this is that it breaks backwards compatibility as some functions return signature will change.

@adam-urbanczyk
Copy link
Member

@voneiden do you need some specific function(s)? If so, I can add a workaround for 7.6.3. Eventually, I'd like to merge your PR or develop something similar, but not in the nearest feature.

@voneiden
Copy link
Author

@adam-urbanczyk I'm good as is at the moment thanks, no need to include temporary workarounds. Parameter_s could be interesting to have available at some point but priority is minimal for me.

@MatthiasJ1
Copy link

Just wanted to bump this issue. Also, following the current implementation I am assuming you would have something like:

int F(int &arg1);

Turning into

def F() -> Tuple[int, int]: ...

Although I don't know if OCCT has any inout pass by reference arguments so maybe

def F(int) -> Tuple[int, int]: ...

would be more appropriate.

An alternative solution which would require less drastic API changes (for better or worse) would be to either support the builtin ctypes solution for this (https://docs.python.org/3/library/ctypes.html#passing-pointers-or-passing-parameters-by-reference) (which for some reason PyBind11 does not support), or to introduce a custom python object which handles this and your code generation could take into account (pseudocode):

class Ref:
  def __init__(self, value): self.value = value
m.def("F", [](py::object arg1_ref) { return F(arg1_ref.value);});
a = Ref(0)
F(a)

@jdegenstein
Copy link

Just ran into this issue while trying to use ShapeAnalysis_Curve().Project() from OCP.ShapeAnalysis

One "overload" is this:

Project(): The following argument types are supported:
    1. (self: OCP.ShapeAnalysis.ShapeAnalysis_Curve, C3D: OCP.Geom.Geom_Curve, P3D: OCP.gp.gp_Pnt, preci: float, proj: OCP.gp.gp_Pnt, param: float, AdjustToEnds: bool = True) -> float
    ...

In this case the parameters: proj and param appear to be pointers and the function return value appears to be the distance between the input point and resulting projected point.

I am not sure if this was previously known, but I am able to partially workaround by creating proj_result = Vector().to_pnt() and then setting ShapeAnalysis_Curve().Project(... proj=proj_result) which allows the gp_Pnt result data to be written into proj_result. I am not aware of a current workaround for param which is of type Standard_Real which I don't think can be created in python since it is mapped to float (AFAIK). In summary, as far as I know it is currently not possible to use the above method at all in OCP so I would really like to know if this can be enhanced.

p.s.
Relevant discussion at pybind11 pybind/pybind11#134 and relevant FAQ https://pybind11.readthedocs.io/en/stable/faq.html#limitations-involving-reference-arguments with a few workarounds (only accessible at the C++ level I think)

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

4 participants