[libcamera-devel] [PATCH 11/14] py: add geometry classes

[Tomi Valkeinen]

On 17/05/2022 12:48, Laurent Pinchart wrote:

>>>> +	py::class_<Rectangle>(m, "Rectangle")
>>>> +		.def(py::init<>())
>>>> +		.def(py::init<int, int, Size>())
>>>> +		.def(py::init<>([](int x, int y, const std::array<unsigned int, 2> &s) {
>>>> +			return Rectangle(x, y, std::get<0>(s), std::get<1>(s));
>>>> +		}))
>>>
>>> I'm puzzled a bit by this constructor. Where do you use it, and could
>>> then next constructor be used instead ? If it's meant to cover a common
>>> use case, should the C++ API also implement this ?
>>
>> It allows:
>>
>> libcam.Rectangle(1, 2, (3, 4))
> 
> And then someone will request being able to write
> 
> 	libcam.Rectangle((1, 2), (3, 4))
> 
> and possibly even
> 
> 	libcam.Rectangle((1, 2), 3, 4)
> 
> :-)

Yep. As a user, I would expect/hope all these would work:

Rectangle(Point(1, 2), Size(3, 4))
Rectangle((1, 2), (3, 4))
Rectangle([1, 2], [3, 4])
Rectangle(size=(3,4))  # pos is (0, 0)
Rectangle(1, 2, 3, 4)  # not sure about this, it's a bit confusing

Managing Point, Size, tuples and lists in the above example is solved by 
expecting an object that gives us two ints, instead of expecting 
something specific. If both Point and Size support __iter__, and tuples 
and lists already do, then:

x,y = pos
w,h = size

will work for all those cases.

And looking at the transforming methods, e.g.:

rect.scale_by(Size(1, 2), Size(3,4)).translate_by(Point(5,6))

is a bit annoying to use, compared to:

rect.scale_by((1, 2), (3,4)).translate_by((5,6))

To fix that, I think we would have to overwrite all the methods we want 
to support such conversions. Which does not sound nice.

I did some testing in the __init__.py, and I think we can (re-)implement 
anything we need there. E.g.:

def __Size_iter(self):
     yield from (self.width, self.height)

Size.__iter__ = __Size_iter


and


def __Rectangle_init(self, pos=(0, 0), size=(0, 0)):
     x, y = pos
     w, h = size
     Rectangle.__old_init(self, x, y, w, h)

Rectangle.__old_init = Rectangle.__init__
Rectangle.__init__ = __Rectangle_init

> In C++ it's a bit different thanks to implicit constructors and
> initializer lists, so I agree that we may want to expose a similar
> feature manually to make the code more readable. I'm just concerned
> about the possible large number of combinations.
> 
> Is there a Python coding style rule (or rather API design rule) about
> this ?

I'm not aware of such. In my experience python classes/functions often 
take a wide range of different inputs, and they do what you expect them 
to do. I don't know if that's recommended or is it just just something 
that the authors have implemented because they liked it.

  Tomi