[PATCH V2] smartset: add a "toset" method

[Pierre-Yves David]

On 06/04/2017 02:55 AM, Martin von Zweigbergk wrote:
>
>
> On Jun 3, 2017 5:02 PM, "Augie Fackler" <raf at durin42.com
> <mailto:raf at durin42.com>> wrote:
>
>
>     > On Jun 3, 2017, at 7:29 PM, Pierre-Yves David
>     <pierre-yves.david at ens-lyon.org
>     <mailto:pierre-yves.david at ens-lyon.org>> wrote:
>     >
>     >>> +    def toset(self):
>     >>> +        return self._set
>     >>
>     >> It freaks me out just a little (maybe too much Rust today?) to
>     leak self._set mutably like this. What do you think of making a
>     copy? Should we just strongly admonish in the “convert to a set”
>     docstring that callers of toset() _must not_ mutate the returned set?
>     >
>     > Note: For the target code, copying sets is can be multiple percent
>     of the total run time.
>
>     Okay, it sounds like we should document that the API contract is
>     that clients must not mutate the returned set. Does that sound
>     workable to everyone?

That works for me (ideally, python would have an API to turn a set into 
a frozen set in place).

Regarding this series, the new function currently do not have any user 
so I think we should wait a bit. Updating the whole API of smartset for 
something that we are not sure to use yet seems premature.

I think Jun made his point in terms of "what he would like the API to 
looks like." and we should get back to that as part of a series actually 
using it (so that we can discuss the benefit).

> Sounds good to me.
>
> But I think the argument about "multiple percent of the total run time"
> is pretty silly when the total run time is about a millisecond (IIUC).

I agree it is not too critical

-- 
Pierre-Yves David