[libcamera-devel] [PATCH] py: Improve print methods for Transform and ColorSpace objects

[Laurent Pinchart]

Hi David,

On Wed, May 18, 2022 at 12:26:06PM +0100, David Plowman wrote:
> On Wed, 18 May 2022 at 11:02, Laurent Pinchart wrote:
> > On Wed, May 18, 2022 at 10:08:01AM +0100, David Plowman via libcamera-devel wrote:
> > > They should now print out their meaningful string representations
> > > instead of "object at <address>".
> > >
> > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > > ---
> > >  src/py/libcamera/pymain.cpp | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
> > > index fb89975c..2ba5fcbb 100644
> > > --- a/src/py/libcamera/pymain.cpp
> > > +++ b/src/py/libcamera/pymain.cpp
> > > @@ -567,6 +567,9 @@ PYBIND11_MODULE(_libcamera, m)
> > >               .def("__str__", [](Transform &self) {
> > >                       return "<libcamera.Transform '" + std::string(transformToString(self)) + "'>";
> > >               })
> > > +             .def("__repr__", [](Transform &self) {
> > > +                     return "<libcamera.Transform '" + std::string(transformToString(self)) + "'>";
> > > +             })
> >
> > Can we do the same as "[PATCH v2 12/13] py: add geometry classes" and
> > return a string that would construct an identical Transform object when
> > run as Python code ? Same below.
> 
> Just to be clear then, you'd prefer a string like (for example)
> 
> libcamera.Transform(hflip=False, vflip=False, transpose=False)
> 
> ?

It's possibly a bit long. I wonder if we should expose the C++ presets.

> > >               .def_property("hflip",
> > >                             [](Transform &self) {
> > >                                     return !!(self & Transform::HFlip);
> > > @@ -617,6 +620,9 @@ PYBIND11_MODULE(_libcamera, m)
> > >               .def("__str__", [](ColorSpace &self) {
> > >                       return "<libcamera.ColorSpace '" + self.toString() + "'>";
> > >               })
> > > +             .def("__repr__", [](ColorSpace &self) {
> > > +                     return "<libcamera.ColorSpace '" + self.toString() + "'>";
> > > +             })
> > >               .def_readwrite("primaries", &ColorSpace::primaries)
> > >               .def_readwrite("transferFunction", &ColorSpace::transferFunction)
> > >               .def_readwrite("ycbcrEncoding", &ColorSpace::ycbcrEncoding)
> 
> And in this case I'd output (for example)
> 
> libcamera.ColorSpace(libcamera.ColorSpace.Primaries.Rec709,
> libcamera.ColorSpace.TransferFunction.Srgb,
> libcamera.ColorSpace.YcbcrEncoding.Rec601,
> libcamera.ColorSpace.Range.Full)
> 
> ?

Ouch, that hurts.

> In this case I could perhaps "optimise" it to
> 
> libcamera.ColorSpace.Jpeg()
> 
> so I'd check for "short versions" first?

That would be good yes, but in the non-standard case, I wonder if such a
long representation is acceptable. Tomi, any opinion ?

-- 
Regards,

Laurent Pinchart