[C++-sig] Re: container suite feedback & ideas

[Raoul Gough]

"Ralf W. Grosse-Kunstleve" <rwgk at yahoo.com> writes:

> Hi Raoul,
>
> Using your documentation I've had no problems wrapping three custom
> linear array types with your container suite.

That's great! Did you have to provide a new Algorithms implementation,
or was a custom ContainerTraits enough? The default_algorithms
template tries to be as undemanding as possible, so that almost
anything with an STL-iterator-style interface should work.

> Just a couple of minor remarks:
>
> static boost::python::indexing::IndexStyle const index_style
>                                 ^
>
> typedef std::map<std::string, int> Container;
>                                    ^
>
> typedef indexing::map_algorithms<simple_map_traits> Algorithms;
>                                                     ^
>
> I believe according to the boost naming conventions these identifiers
> must be all lower case. But I am not really an authority in this
> matter. David?

You're absolutely right here. I thought I'd already cleared out the
mis-placed MixedCaseIdentifiers, but I obviously missed a few. Thanks
for pointing these out.

>
>> #include "algo_selector.hpp"
>> #include "visitor.hpp"
>
> Should probably be #include <boost/...>. It wasn't clear to me
> what I had to include.

Well, I started the documentation when the headers were all in a
private directory outside of the boost tree, so that was how I used to
include them. I'll fix this in the documentation (it was already fixed
in the real headers).

> Eventually just including
>
> #include <boost/python/suite/indexing/container_suite.hpp>
>
> worked for my purposes. Is this what I should be doing?
> Should this be added to <boost/python.hpp>?

Yes, that's what the documentation should have said. I would tend not
to include it from boost/python.hpp, for two reasons.  Firstly, it is
in the "suite" subdirectory, and I would guess that implies that it
currently has the status of an optional add-on. Secondly, it
(indirectly) includes all of the STL container headers (<map>, <set>,
<vector>, etc.) which makes for a big compile-time hit if you don't
otherwise need them. This could be avoided by splitting algo_selector
into multiple parts, and requiring the user to include headers like
<.../indexing/map_support.hpp> and <.../indexing/vector_support.hpp>
to get automatic algorithm selection for those containers.
Alternatively, algo_selector could try to forward-declare the
container templates itself, but I don't know if this is such a good
idea.

>
> There are two important features I am wondering about:
>
> 1. Just doing this
>
>    class_<my_container>("my_container")
>      .def(indexing::container_suite<my_container, my_algorithms>())
>    ;
>
>    doesn't enable me to initialize the instances with Python
>    lists or tuples. Have you thought about this already?
>    My first idea would be to define custom rvalue converters from
>    Python sequences to the wrapped container type
>    (http://www.boost.org/libs/python/doc/v2/faq.html#question2).
>    Then one could pass a Python sequence anywhere the wrapped container
>    is used as a const& or by-value argument. Simply define Python
>    bindings for the C++ copy constructor to enable initialization with
>    Python sequences.

The first thing I notice from your current code is that I should have
been using PyObject_GetIter myself in the python_iterator class,
instead of manually checking for an __iter__ or __getitem__ method.

I hadn't thought about constructing directly from a Python sequence
before, but I think it raises some interesting issues. BTW, if your
container supports slices, what you already *can* do is this:

>>> Vector v
>>> v[:] = [1,2,3]

-or-

>>> v.extend ([1,2,3])

Of course, that's not quite the same thing as construction from a
Python sequence. However, should such a conversion happen implicitly?
I assume the custom rvalue conversions happen automatically, so code
like this would work as follows:

>>> Vector v ([1,2,3])

Boost.Python sees the vector<int>::vector(vector<int> const &) copy
constructor, decides to construct an rvalue from the Python list using
the custom converter and passes this to the copy constructor. So two
copies happen here. It would be ggod if we could tie into the
templated iterator-pair constructor of the container, passing in
something that terates over the Python sequence. This might be
possible using some kind of magic precall policy that would turn a
single Python iterable object into two separate arguments. I'm not
sure exactly how this would work, though.

The other question is how to recognise when the conversion should take
place. For instance, should it only be considered for built-in Python
sequences, or should iterable user-defined containers also work?

>
> 2. Pickling of the wrapped types. This is a big issue but I find
>    it absolutely essential. I have a fairly general solution
>    that I'd be happy to share and extend if there is an interest.
>    If you are curious, it is a three-layer system:
>    The low-level serialization code for built-in types is here:
>     
> http://cvs.sourceforge.net/viewcvs.py/cctbx/scitbx/include/scitbx/serialization/
>    This is designed to maximize portability but is also very fast.
>    The second layer is here:
>     
> http://cvs.sourceforge.net/viewcvs.py/cctbx/scitbx/include/scitbx/boost_python/
>    pickle_single_buffered.h and pickle_double_buffered.h.
>    A top layer for a particular array type is here:
>     
> http://cvs.sourceforge.net/viewcvs.py/cctbx/scitbx/include/scitbx/array_family/boost_python/
>    flex_pickle_single_buffered.h, flex_pickle_double_buffered.h.
>    The system also works for user-defined types (the user has
>    to supply some code).
>    Adapting this system for the container suite would probably
>    require significant changes only to the top layer.

I don't have any experience with Python pickles. If it is simply a
matter of defining a new method for the container, you might be able
to do this via the visitor_helper hook in the Algorithms argument
supplied to container_suite (or directly to indexing::visitor). Is it
that easy, or does the container itself have to provide extra support
features? I suppose the de-pickling phase is also closely related to
the construction issue.

-- 
Raoul Gough.
(setq dabbrev-case-fold-search nil)