[Qgis-developer] SIP and transfer of ownership

[Chris Crook]

Hi Martin

Sorry for the equally long delay, and thanks for your response.   At the moment I don't have time to think about this in any case.

However I agree with your general point, better to address the issue at the C++ level rather than the SIP level, so that everyone benefits and the code base is more robust against ownership oversights.  It does make it a slightly bigger job!

I guess the ideal thing is to document a preferred pattern for implementing these types of objects which are generally copied and not modified (I'm not that familiar with code patterns or what is available in the Qt framework, though I think this sort of pattern is well used within it, eg strings?).

Thanks
Chris


> -----Original Message-----
> From: Martin Dobias [mailto:wonder.sk at gmail.com]
> Sent: Saturday, 8 November 2014 8:08 p.m.
> To: Chris Crook
> Cc: qgis-developer at lists.osgeo.org
> Subject: Re: [Qgis-developer] SIP and transfer of ownership
>
> Hi Chris
>
> (sorry for the long delay!)
>
> You are raising good points.
>
> On Sun, Oct 12, 2014 at 1:35 AM, Chris Crook <ccrook at linz.govt.nz> wrote:
> >
> >     void setSourceColorRamp( QgsVectorColorRampV2* ramp /Transfer/ );
> >
> > So this could be used from python with code something like
> >
> >     ramp=myColorRamp()
> >     renderer.setSourceColorRamp(ramp)
> >
> > However if you were using the same ramp in two places, then you'd need
> > to make sure that you didn't pass the same object to both, so you
> > might have something like
> >
> >    ramp=myColorRamp()
> >    renderer1.setSourceColorRamp(ramp.clone())
> >    renderer2.setSourceColorRamp(ramp.clone())
> >
> > Is this right?
>
> Yes. But you could as well do this:
>
>     renderer1.setSourceColorRamp(ramp.clone())
>     renderer2.setSourceColorRamp(ramp)
>
>
> > If so, then I wonder if having to pay attention to this sort of memory
> allocation/ownership issue is not very "pythonic".
>
> Indeed it is not very Pythonic. Qt library is able to avoid such issues much
> better. Why is that: when /Transfer/ is needed (e.g. in calls involving
> QObject-based classes), the classes explicitly remember their
> parents/children. So even if you try to assign one object to two different
> parents (like above), it will be actually moved from one parent to another.
> QGIS API does not handle it and that's why we have these issues. A solution
> would be to make internal copies of objects in functions like
> setSourceColorRamp(). That would solve the issue, but it will add an extra
> copy we didn't want. There is solution for that either - to use implicit sharing.
> If the color ramp class supported implicit sharing, the copy would be just
> shallow (no memory copying) - and it would be a win.
>
> I think for a future major release of QGIS we could go through the API and
> see where we have these problems - and ideally fix them as suggested
> above. However the implicit sharing has also its downsides - the
> implementation is slightly more complex and it adds a little bit of overhead.
> But it's nothing we couldn't handle :-)
>
>
> > Would it be better to set up the SIP so that it always used the clone()
> function, and so that a python coder didn't have think about whether they
> might have yielded ownership of a python object.  Presumably there is a way
> to write the SIP to do this?  And would this create issues in those
> environments (eg windows 64) where the transfer function doesn't seem to
> work reliably, and python subclassed objects need to be deliberately kept
> alive?
>
> I don't think that trying to solve this at SIP level would be good.
> Any deviation between C++ and Python API (in terms of some special code
> for SIP) is a pain, so if possible let's address that in C++ code (with
> improvements for both C++ and Python devs). Also from C++ developer's
> point of view, it is much nicer if you do not need to think how the ownership
> is handled in each such method.
>
> Cheers
> Martin

This message contains information, which may be in confidence and may be subject to legal privilege. If you are not the intended recipient, you must not peruse, use, disseminate, distribute or copy this message. If you have received this message in error, please notify us immediately (Phone 0800 665 463 or info at linz.govt.nz) and destroy the original message. LINZ accepts no responsibility for changes to this email, or for any attachments, after its transmission from LINZ. Thank You.