Better constructor for Segmentation

[larsmans]

In the spirit of dea63de, here's an (IMHO) cleaner and more Pythonic way to construct a Segmentation object: a proper constructor instead of a factory function on a different class and a bunch of setters (ugh). Introducing a factory function on PointCloud for each and every other class is introducing so much coupling that in the long run it's just not going to work.

I'm doing my best to preserve compatibility with these changes, but the factory functions are tying down our implementation choices. I'd love to split off Segmentation into a separate module and implement the constructor in Python to keep build times and object sizes down, i.e.

# _segmentation.pyx
cdef class _Segmentation:
    # low-level C++ stuff

# segmentation.py
from ._segmentation import _Segmentation

class Segmentation(_Segmentation):
    # high-level, pure Python stuff like __init__

... but currently that would lead to circular dependencies because PointCloud.make_segmenter needs to know about Segmentation, and vice-versa Segmentation needs to know about PointCloud.

---

This change is 

[nzjrs]

Hi Lars,

Just letting you know I have visitors and holidays for the next 2 weeks, so
I won't be able to properly look at this.

One approach I sometimes use to maintain some backward compatibility is to
write a $module_name/compat.py and do all the monkey patching in python, in
there.

Other than that I agree that getting to a simpler more pythonic api is a
good thing.

John
On 14/07/2014 4:05 PM, "Lars Buitinck" [email protected] wrote:

In the spirit of dea63de
dea63de,
here's an (IMHO) cleaner and more Pythonic way to construct a Segmentation
object: a proper constructor instead of a factory function on a different
class and a bunch of setters (ugh). Introducing a factory function on
PointCloud for each and every other class is introducing so much coupling
that in the long run it's just not going to work.

I'm doing my best to preserve compatibility with these changes, but the
factory functions are tying down our implementation choices. I'd love to
split off Segmentation into a separate module and implement the
constructor in Python to keep build times and object sizes down, i.e.

_segmentation.pyx

cdef class _Segmentation:
# low-level C++ stuff

segmentation.py

from ._segmentation import _Segmentation

class Segmentation(_Segmentation):
# high-level, pure Python stuff like init

... but currently that would lead to circular dependencies because
PointCloud.make_segmenter needs to know about Segmentation, and

vice-versa Segmentation needs to know about PointCloud.

You can merge this Pull Request by running

git pull https://github.com/larsmans/python-pcl constructors

Or view, comment on, or merge it at:

#39
Commit Summary

• fix repr on PointClouds
• "except +" for minipcl functions
• remove implementation detail from docstring
• better contructor + documentation for Segmentation

File Changes

• M pcl/_pcl.pyx
  https://github.com/strawlab/python-pcl/pull/39/files#diff-0 (85)
• M readme.rst
  https://github.com/strawlab/python-pcl/pull/39/files#diff-1 (4)
• M tests/test.py
  https://github.com/strawlab/python-pcl/pull/39/files#diff-2 (7)

Patch Links:

• https://github.com/strawlab/python-pcl/pull/39.patch
• https://github.com/strawlab/python-pcl/pull/39.diff

—
Reply to this email directly or view it on GitHub
#39.

[oroulet]

Is this really an improvment? using string as identifiers is bug prone and deacreses readability.
Would be better to put the model names in an enum og simply a class