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

Idea: Make Handle_* classes completely transparent #539

Closed
rainman110 opened this issue May 30, 2018 · 3 comments
Closed

Idea: Make Handle_* classes completely transparent #539

rainman110 opened this issue May 30, 2018 · 3 comments

Comments

@rainman110
Copy link
Collaborator

Using the GetHandle(), GetObject() functions is awkward from a python perspective.

Users should not know the details of reference counted objects, when dealing with pythonocc.

Swig offers typemaps for std::shared_ptr, which wraps them completely transparent. The same can be done for Handle_ classes as well.

Since there is already much code out there, that uses the GetHandle() GetObject() syntax, we don't want to break this code. Therefore, both functions can be added, returning just a self reference and doing no harm.

I created a proof of concept, which shows that this in pricipally possible:
https://github.com/rainman110/swighandles

The file https://github.com/rainman110/swighandles/blob/master/standard.i shows, how easy it would be to wrap the handle classes. The dark magic obviously happens at https://github.com/rainman110/swighandles/blob/master/occ_handle1.i.

Much of the code and ideas are actually borrowed from std_shared_ptr.i from swig!

@jf--- @tpaviot What do you think about this idea?

@jf---
Copy link
Contributor

jf--- commented May 31, 2018

What do you think about this idea?

That would be a huge deal, in terms of elegance of API, which seems obvious.
But out of my depth to comment on the technical aspects / maintainability.
A thrilling perspective @rainman110 !

@rainman110
Copy link
Collaborator Author

Hi together. I have a first semi-working code. Most of the examples already work.

What currently does not work are the following issues:

  • There are no Handle_ classes anymore. This means, that all checks is_instance(obj, Handle_Geom_Curve) do not work. They now have to be is_instance(obj, Geom_Curve). A workaround would be, to simply make an alias from e.g. Handle_Geom_Curve to Geom_Curve in Python.
  • I had to disable all Classes that derive from Standard_Transient in the end but which don't have a Handle class defined. Either, we simply create a Handle class for them, or, we find another workaround. This is mostly the case for the AIS package. E.g. AIS_Textured_Shape does not have a handle right now.
  • There are yet no downcast methods since I completely removed the Handle_* classes from the interface files. It is principally simple to re-add them though.

@rainman110
Copy link
Collaborator Author

rainman110 commented Jun 15, 2018

@jf--- Okay... Now I managed to solve all the problems! 😄

All examples are now working, meaning I achieved full backwards compatibilty.

Compared to the old code, one can now implement much more pythonic code like:

  • Before, we had to check Handles using the "IsNull()" method.
  • Now, you can simply check "is None"
  • You don't need any Handle_* classes any more
  • You don't need GetObject(), GetHandle() methods anymore

I would create some conda packages, such that you can test it with your code. My adaptions are currently based on pythonocc-core 0.17.2 though.

rainman110 added a commit to rainman110/pythonocc-core that referenced this issue Nov 15, 2018
The OCCT handle smart pointer API is a C++ specific detail
and should not be exposed to Python. Instead it should be
completely invisible.
I used the std::shared_ptr swig library as a basis for the
implementation.
To be compatible with most of the "old" code, GetHandle() and
GetObject() methods still exist but do nothing except from
a deprecation warning. In general, I tried to make sure
that the code is backwards compatible as possible.

Closes tpaviot#539
rainman110 added a commit to rainman110/pythonocc-core that referenced this issue Nov 16, 2018
The OCCT handle smart pointer API is a C++ specific detail
and should not be exposed to Python. Instead it should be
completely invisible.
I used the std::shared_ptr swig library as a basis for the
implementation.
To be compatible with most of the "old" code, GetHandle() and
GetObject() methods still exist but do nothing except from
a deprecation warning. In general, I tried to make sure
that the code is backwards compatible as possible.

Closes tpaviot#539
tpaviot pushed a commit that referenced this issue Nov 17, 2018
The OCCT handle smart pointer API is a C++ specific detail
and should not be exposed to Python. Instead it should be
completely invisible.
I used the std::shared_ptr swig library as a basis for the
implementation.
To be compatible with most of the "old" code, GetHandle() and
GetObject() methods still exist but do nothing except from
a deprecation warning. In general, I tried to make sure
that the code is backwards compatible as possible.

Closes #539
tpaviot pushed a commit that referenced this issue Nov 18, 2018
The OCCT handle smart pointer API is a C++ specific detail
and should not be exposed to Python. Instead it should be
completely invisible.
I used the std::shared_ptr swig library as a basis for the
implementation.
To be compatible with most of the "old" code, GetHandle() and
GetObject() methods still exist but do nothing except from
a deprecation warning. In general, I tried to make sure
that the code is backwards compatible as possible.

Closes #539

Updated SWIG files
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

2 participants