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

[Laurent Pinchart]

Hi Tomi,

On Tue, May 17, 2022 at 05:17:46PM +0300, Tomi Valkeinen wrote:
> On 17/05/2022 16:56, Laurent Pinchart wrote:
> > On Tue, May 17, 2022 at 02:27:25PM +0100, David Plowman wrote:
> >> Hi everyone
> >>
> >> I see there's been an interesting discussion going on. I'm just going
> >> to toss in my tuppence worth without coming to any firm conclusions,
> >> so I apologise in advance!
> >>
> >> * Most importantly libcamera's Python bindings need to expose the
> >> basic libcamera C++ API. I hope that's uncontroversial!
> >>
> >> * Generally I'm not convinced it's worth worrying too much about how
> >> "friendly" the Python API is. I'm not saying we should deliberately
> >> make it difficult, but I think the libcamera API is too intimidating
> >> for casual camera users ("help! I just want to capture a picture!") or
> >> even those developing Python applications who would probably
> >> appreciate something higher level ("I just want clicking on this
> >> button to start the camera!").
> >>
> >> * So I wouldn't do lots of work or add lots of features to try and
> >> achieve that, though making it Pythonic and easy-to-use wherever we
> >> can is of course still desirable.
> > 
> > That seems reasonable, we can start by staying close to the C++ API with
> > a minimum amount of "pythonic" rework of the API, and then improve on
> > top later.
> > 
> >> * I think there's maybe an argument for a "friendly" and slightly
> >> higher level (?) libcamera Python API on top of the basic one (a bit
> >> like Picamera2 is for us). But maybe there should be a distinction?
> >> Not sure.
> >>
> >> * I'm not sure what I think of providing all the geometry classes. On
> >> one hand there's no harm in it, on the other... wouldn't most Python
> >> programmers prefer to deal with tuples?
> 
> The geometry classes do offer quite many transformation functions. 
> Having the classes also makes the bindings simpler, as we don't need to 
> convert between tuples and the proper geometry class in multiple places 
> (see the "py: use geometry classes").
> 
> > I can't tell about "most Python programmers", but I find
> > 
> > 	foo(Size(100, 100), Size(500, 500))
> > 
> > more readable than
> > 
> > 	foo((100, 100), (500, 500))
> 
> For "foo", I agree, as I have no clue what it does ;).
> 
> For Rect, I agree also, as you can well have Rect(Point, Point) and 
> Rect(Point, Size).
> 
> I was a bit surprised to find Rectangle(int, int, uint, uint) 
> constructor on the C++ side, as it's not obvious if the latter uints are 
> for a Point or Size. It also doesn't have Rectangle(Point, Size) at all.

You're right about that. At least the latter should be fixed, and
possibly the former too.

> In any case, I've dropped the "friendly" code from the series for now.
> 
> > as the latter doesn't clearly say if we're dealing with points or sizes
> > (or something else). I don't know if that's relevant, but
> > https://www.riverbankcomputing.com/static/Docs/PyQt5/api/qtcore/qrect.html
> > doesn't provide a constructor that takes two lists or tuples.
> 
> I don't think Qt is very pythonic.

That's a fair point, and maybe why my APIs are more "Qt-ic" than
pythonic :-)

-- 
Regards,

Laurent Pinchart