cwi-dis / cwipc

MIT License
15 stars 2 forks source link

Make Python API more Pythonic, and make python-only use simpler #92

Open jackjansen opened 6 months ago

jackjansen commented 6 months ago

The main issue is the .free() paradigm, i.e. the fact that freeing the Python object doesn't free the underlying point cloud (or other object).

This needs to be reversed: default behaviour should be that the native object is freed when the Python refcount drops to zero.

As an alternative there should be a .detach() method, which will invalidate the object on which it is called without freeing the underlying native object. It will return a new Python object referring to the same underlying native object, but with a flag set that freeing the Python object should do nothing to the underlying native object.

The Python package should be made available on pypi, including the essential native dynamic libraries. It is probably a good idea if these do not include the Realsense and Kinect DLLs, but reference system-installed versions of those (and print understandable error messages when the libraries have not been installed yet).

jackjansen commented 6 months ago

We should also rename the Python methods and objects, the use of cwpic as both the toplevel package name and the point cloud object name is bothersome. The object should probably be named Pointcloud.

Also, the wrapper modules (especially cwipc_util) have become rather heavy. I think the cwipc_util module should really only contain the wrapper code to the native objects (which would then be something like cwipc.cwipc_util_wrapper.Pointcloud_wrapper) and these should be given additional functionality at the toplevel, through a class cwipc.Pointcloud which is a subclass of the previous one, with the extra Pythonic functionality added.