Re: [C++-sig] Registration order in pyplusplus

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

Niall Douglas
As requested moved onto the python C++ SIG mailing list.

> > Tried it with CVS pyplusplus this morning and it's still not ordering
> > registrations correctly. If you try loading in the TnFOX.pyd module
> > you will get the "No to_python (by-value) converter found for C++
> > type: class FX::FXACLEntity" fatal exception because
> > register_FXACL_class() needs register_FXACLEntity_class() before it.
>
> This is what I don't understand and can not create with simple use case,
> that will reproduce the problem.
>
> See this mail:
> http://mail.python.org/pipermail/c++-sig/2006-January/010100.html

This email is incorrect. Here is the backtrace for when the exception
is thrown:

  TnFOX.dll!boost::python::throw_error_already_set()  Line 61 C++

  TnFOX.dll!boost::python::converter::registration::to_python(const
void * source=0x605dc654)  Line 53 C++

TnFOX.dll!boost::python::converter::detail::arg_to_python_base::arg_to
_python_base(const void * source=0x605dc654, const
boost::python::converter::registration & converters={...})  Line 24 +
0x13 C++
 
TnFOX.dll!boost::python::api::object_initializer_impl<0,0>::get<FX::FX
ACLEntity>(const FX::FXACLEntity & x={...}, boost::mpl::bool_<0>
__formal={...})  Line 368 + 0x12 C++
 
TnFOX.dll!boost::python::api::object_base_initializer<FX::FXACLEntity>
(const FX::FXACLEntity & x={...})  Line 290 + 0xd C++
 
TnFOX.dll!boost::python::detail::keywords<1>::operator=<FX::FXACLEntit
y>()  Line 74 + 0x9 C++

        TnFOX.dll!register_FXACL_class()  Line 285 + 0x54 C++

  TnFOX.dll!init_module_TnFOX()  + 0x19 C++


This localises the problem code to:

void register_FXACL_class(){
    if( true ){
        typedef bp::class_< FXACL_wrapper > FXACL_exposer_t;
        FXACL_exposer_t FXACL_exposer = FXACL_exposer_t( "FXACL",
bp::init< FX::FXACL::EntityType, FX::FXACLEntity const & >((
bp::arg("type")=::FX::FXACL::Unknown,
bp::arg("owner")=FX::FXACLEntity::currentUser( ) ))/*[ undefined call
policies ]*/ );

In other words, the requirement for the FXACLEntity definition to BPL
is coming from its use in the constructor for FXACL - specifically
the bit ' bp::arg("owner")= ' - which I am guessing from the
boost::python::detail::keywords<1>::operator=<FX::FXACLEntity>() just
above it in the backtrace.

Dave - does boost::python::arg()= require the type of the parameter
being set to be already registered? I would imagine that it surely
does.

> I attach test file, can you take a look on them? May be you have some ideas?
> What can cause this error?

Really, the best thing is to build the full TnFOX Python bindings and
test them. It's probably a little slow on your computer though, so I
understand if you don't want to do so.

> > > > There's a simple solution - ensure that registrations occur in
> > > > exactly the same order of class definitions in the parsed header. I
> > > > vaguely remember a problem with this as GCCXML doesn't always do what
> > > > one expect here, but you're more an expert here than I.
> > >
> > > pyplusplus has class class_organizer_t that is responsible for order of classes.
> > > It uses topological sort in order to decide it. If needed, I can add
> > > an other condition
> > > to it.
> >
> > If you examine the fx.xml file you will see that FXACLEntity is given
> > id=_27958 and FXACL is given id=_28827. Now I don't know if the id
> > number is incremented linearly, but I do notice that incomplete
> > declarations can happen and these would screw with the correct
> > ordering. You need to order registrations in the same order as
> > *complete* declarations. This may mean running your own counter which
> > is only incremented with every complete declaration, then later
> > ordering in terms of this count.
>
> Niall, I don't understand what cause the problem. So I can not solve it.
> I need to understand it first. After this I am sure we can find some solution.

I had the same problem with pyste, hence Bruno implemented the
solution I suggested above and it worked.

> I do not want to prevent you from working. I give you few ideas for
> work around:

It's okay - I have an obscene amount of work on right now (I am
organising a lecture series in addition to my normal workload, and it
begins next week).

> 1. exclude those class
>     very simple solution, you can implement this in few minutes,
>
> 2. change the order of classes: you can find FXACL and FXACLEntity code creators
>    and to adjust their positions. I can implement this solution for
> you. Tell me if you
>    want me to do it for you. If you want to try to implement it
> itself, then take a look
>    on code_creators.compound_t class. It has to members remove_creator and
>    adopt_creator. adopt_creator allows you to place creator to
> specific position.
>    code_creators.creator_finder class provides functionality to find
> code creator
>    base on some criteria.

If you try either of these you run into major problems - there are
too many dependency issues to do it manually. I did try shuffling
around the registrations but gave up after I had moved ten.

Cheers,
Niall



_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

Roman Yakovenko
On 2/8/06, Niall Douglas <[hidden email]> wrote:
> As requested moved onto the python C++ SIG mailing list.

Thanks

> > > Tried it with CVS pyplusplus this morning and it's still not ordering
> > > registrations correctly. If you try loading in the TnFOX.pyd module
> > > you will get the "No to_python (by-value) converter found for C++
> > > type: class FX::FXACLEntity" fatal exception because
> > > register_FXACL_class() needs register_FXACLEntity_class() before it.
> >
> > This is what I don't understand and can not create with simple use case,
> > that will reproduce the problem.
> >
> > See this mail:
> > http://mail.python.org/pipermail/c++-sig/2006-January/010100.html
>
> This email is incorrect. Here is the backtrace for when the exception
> is thrown:
>
>         TnFOX.dll!boost::python::throw_error_already_set()  Line 61     C++
>
>         TnFOX.dll!boost::python::converter::registration::to_python(const
> void * source=0x605dc654)  Line 53      C++
>
> TnFOX.dll!boost::python::converter::detail::arg_to_python_base::arg_to
> _python_base(const void * source=0x605dc654, const
> boost::python::converter::registration & converters={...})  Line 24 +
> 0x13    C++
>
> TnFOX.dll!boost::python::api::object_initializer_impl<0,0>::get<FX::FX
> ACLEntity>(const FX::FXACLEntity & x={...}, boost::mpl::bool_<0>
> __formal={...})  Line 368 + 0x12        C++
>
> TnFOX.dll!boost::python::api::object_base_initializer<FX::FXACLEntity>
> (const FX::FXACLEntity & x={...})  Line 290 + 0xd       C++
>
> TnFOX.dll!boost::python::detail::keywords<1>::operator=<FX::FXACLEntit
> y>()  Line 74 + 0x9     C++
>
>         TnFOX.dll!register_FXACL_class()  Line 285 + 0x54       C++
>
>         TnFOX.dll!init_module_TnFOX()  + 0x19   C++
>
>
> This localises the problem code to:
>
> void register_FXACL_class(){
>     if( true ){
>         typedef bp::class_< FXACL_wrapper > FXACL_exposer_t;
>         FXACL_exposer_t FXACL_exposer = FXACL_exposer_t( "FXACL",
> bp::init< FX::FXACL::EntityType, FX::FXACLEntity const & >((
> bp::arg("type")=::FX::FXACL::Unknown,
> bp::arg("owner")=FX::FXACLEntity::currentUser( ) ))/*[ undefined call
> policies ]*/ );

Thank you very much, now I understand the whole problem. See end of the email
for solution.

> In other words, the requirement for the FXACLEntity definition to BPL
> is coming from its use in the constructor for FXACL - specifically
> the bit ' bp::arg("owner")= ' - which I am guessing from the
> boost::python::detail::keywords<1>::operator=<FX::FXACLEntity>() just
> above it in the backtrace.
>
> Dave - does boost::python::arg()= require the type of the parameter
> being set to be already registered? I would imagine that it surely
> does.

I will start an other thread. I have simple test case that reproduce the problem

> Really, the best thing is to build the full TnFOX Python bindings and
> test them. It's probably a little slow on your computer though, so I
> understand if you don't want to do so.

I do want but I am not able. pyplusplus generates +- 300 cpp files. It
takes  2.5 minutes
to compile every file on my laptop at home. So in order to compile
TnFOX I need more or
less 12-13 hours. On my work we have compilation farm from 15
computers. So it takes
10 - 15 minutes to compile it. But I do not have time to setup whole
environment.
And the main reason: I want to setup unit test for pyplusplus.

>
> > > > > There's a simple solution - ensure that registrations occur in
> > > > > exactly the same order of class definitions in the parsed header. I
> > > > > vaguely remember a problem with this as GCCXML doesn't always do what
> > > > > one expect here, but you're more an expert here than I.
> > > >
> > > > pyplusplus has class class_organizer_t that is responsible for order of classes.
> > > > It uses topological sort in order to decide it. If needed, I can add
> > > > an other condition
> > > > to it.
> > >
> > > If you examine the fx.xml file you will see that FXACLEntity is given
> > > id=_27958 and FXACL is given id=_28827. Now I don't know if the id
> > > number is incremented linearly, but I do notice that incomplete
> > > declarations can happen and these would screw with the correct
> > > ordering. You need to order registrations in the same order as
> > > *complete* declarations. This may mean running your own counter which
> > > is only incremented with every complete declaration, then later
> > > ordering in terms of this count.
> >
> > Niall, I don't understand what cause the problem. So I can not solve it.
> > I need to understand it first. After this I am sure we can find some solution.
>
> I had the same problem with pyste, hence Bruno implemented the
> solution I suggested above and it worked.

I am sure this solution works. I have only one problem with this
solution: it works
only in case you want to export declarations, from single header file.

Solution for the problem:
function_t and constructor_t code creators has property "use_keywords"
This property is responcible for generation of keyword arguments. If you set
it to False, then keyword arguments will not be generated.

Here is relevant code:

relevant_classes = (code_creators.function_t, code_creators.constructor_t )
calldefs = filter( lambda creator: isinstance( creator, relevant_classes )
                       , code_creators.make_flatten( extmodule.creators ) )
  for calldef in calldefs:
     calldef.use_keywords = False

Your version of pyplusplus already has this functionality, but it has few bug.
Please, use latest CVS version.

> Cheers,
> Niall
>

Thanks.

--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/
_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

Niall Douglas
On 9 Feb 2006 at 7:59, Roman Yakovenko wrote:

> I am sure this solution works. I have only one problem with this
> solution: it works
> only in case you want to export declarations, from single header file.

Indeed. Pyste only worked correctly when you did this because of this
exact reason.

> Solution for the problem:
> function_t and constructor_t code creators has property "use_keywords"
> This property is responcible for generation of keyword arguments. If you set
> it to False, then keyword arguments will not be generated.

Seems a shame. Pyste never told python about default arguments, so if
you asked for help on that API from within python you couldn't see
what the defaults were. Furthermore, later on, I'm sure you will want
to integrate docstrings pulled from doxygen output and if the
parameter names are not known you won't be able to print per-
parameter (\param) docs.

> Here is relevant code:
>
> relevant_classes = (code_creators.function_t, code_creators.constructor_t )
> calldefs = filter( lambda creator: isinstance( creator, relevant_classes )
>                        , code_creators.make_flatten( extmodule.creators ) )
>   for calldef in calldefs:
>      calldef.use_keywords = False
>
> Your version of pyplusplus already has this functionality, but it has few bug.
> Please, use latest CVS version.

No chance of sorting based on arg() usage being added?

Your other email where arg() doesn't need predefinition is equally a
solution. BPL could lazily add in the type info later, but it would
involve maintaining a list of where that type has been used in an
arg() and retrospectively linking it in when the definition is made.
I have a feeling that Dave won't want that extra complexity when you
can just reorder the registrations.

Cheers,
Niall



_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

Roman Yakovenko
On 2/9/06, Niall Douglas <[hidden email]> wrote:
> Seems a shame. Pyste never told python about default arguments, so if
> you asked for help on that API from within python you couldn't see
> what the defaults were. Furthermore, later on, I'm sure you will want
> to integrate docstrings pulled from doxygen output and if the
> parameter names are not known you won't be able to print per-
> parameter (\param) docs.

Yes. I definitely will do it.

> No chance of sorting based on arg() usage being added?

I will do it. But it will take some time. 19 - 28 I have a trip to Dead Sea.
So I do not want to insert this functionality before it. Also CVS version
is much better then previous released one. I'd like to make a release
before I implement this. Work around I provided you is temporal.
You can continue to work, me too. After I implement the functionality
the only thing that you will have to do is delete few lines of code.

> Your other email where arg() doesn't need predefinition is equally a
> solution. BPL could lazily add in the type info later, but it would
> involve maintaining a list of where that type has been used in an
> arg() and retrospectively linking it in when the definition is made.
> I have a feeling that Dave won't want that extra complexity when you
> can just reorder the registrations.

I will explain my point in that thread.

> Cheers,
> Niall
>

Thanks

--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/
_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

Niall Douglas
In reply to this post by Roman Yakovenko
On 9 Feb 2006 at 7:59, Roman Yakovenko wrote:

> Solution for the problem:
> function_t and constructor_t code creators has property "use_keywords"
> This property is responcible for generation of keyword arguments. If you set
> it to False, then keyword arguments will not be generated.
>
> Here is relevant code:
>
> relevant_classes = (code_creators.function_t, code_creators.constructor_t )
> calldefs = filter( lambda creator: isinstance( creator, relevant_classes )
>                        , code_creators.make_flatten( extmodule.creators ) )
>   for calldef in calldefs:
>      calldef.use_keywords = False
>
> Your version of pyplusplus already has this functionality, but it has few bug.
> Please, use latest CVS version.

I updated to CVS, but the use_keywords property is already False by
default and furthermore, there is no code in _keywords_args() to test
the state of use_keywords and generate alternative code
appropriately.

Did you commit your changes to CVS?

Cheers,
Niall



_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

Roman Yakovenko
On 2/9/06, Niall Douglas <[hidden email]> wrote:
> On 9 Feb 2006 at 7:59, Roman Yakovenko wrote:
> I updated to CVS, but the use_keywords property is already False by
> default and furthermore, there is no code in _keywords_args() to test
> the state of use_keywords and generate alternative code
> appropriately.
>
> Did you commit your changes to CVS?

Yes I do. May be it takes some time?
Any way I attach relevant file. Please, place it to code_creators package.

> Cheers,
> Niall
>
>
>
> _______________________________________________
> C++-sig mailing list
> [hidden email]
> http://mail.python.org/mailman/listinfo/c++-sig
>

--
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/

_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig

calldef.py (39K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

Niall Douglas
On 9 Feb 2006 at 14:18, Roman Yakovenko wrote:

> > Did you commit your changes to CVS?
>
> Yes I do. May be it takes some time?
> Any way I attach relevant file. Please, place it to code_creators package.

Obviously it takes time to propogate on sourceforge. God I hate CVS.
Bring on subversion!

You're still missing something - where currently it outputs:

        FXACL_exposer_t FXACL_exposer = FXACL_exposer_t( "FXACL",
bp::init< FX::FXACL::EntityType, FX::FXACLEntity const & >()/*[
undefined call policies ]*/ );

... it should be:

        FXACL_exposer_t FXACL_exposer = FXACL_exposer_t( "FXACL",
bp::init< bp::optional< FX::FXACL::EntityType, FX::FXACLEntity const
& > >()/*[ undefined call policies ]*/ );

... because otherwise you must always specify construction parameters
even when defaults are available.

I think this is a very easy fix.

Cheers,
Niall



_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig
Reply | Threaded
Open this post in threaded view
|

Re: [C++-sig] Registration order in pyplusplus

David Abrahams
In reply to this post by Niall Douglas
"Niall Douglas" <[hidden email]> writes:

> Dave - does boost::python::arg()= require the type of the parameter
> being set to be already registered? I would imagine that it surely
> does.

Yes, the RHS is converted then and there, so there needs to be a
conversion available.

--
Dave Abrahams
Boost Consulting
www.boost-consulting.com

_______________________________________________
C++-sig mailing list
[hidden email]
http://mail.python.org/mailman/listinfo/c++-sig