[C++-sig] V2: wrapping int/double bug(?)

[Pearu Peterson]

On Mon, 13 May 2002, David Abrahams wrote:

> ----- Original Message -----
> From: "Pearu Peterson" <pearu at cens.ioc.ee>
>
> > Actually, it was also a problem in Boost.Python v1 and then I solved this
> > issue by not using methods with int or float arguments but using
> > methods with PyObject* arguments and so avoiding the "smart" behaviour of
> > BPL and implementing conversion rules to my particular needs explicitly.
> 
> So, I take it there is a reason that a single overload using a double
> argument doesn't suit your needs? 

Yes, there is. As I mentioned in my previous message, the library that I
am wrapping is a symbolic algebra library where floats are inexact and
ints are exact numbers, roughly speaking. Trivial example: if I'd used
only double argument constructor:

  numeric(double)

then in Python

  numeric(2)/3

becomes inexact

  0.66666666666666663

while the library is designed to handle the above ratio as an exact 
rational number

  2/3

If I'd used only int argument constructor

  numeric(int)

then in python

  numeric(2.4)

becomes wrong

  2

without any complaints.

> I ask because in general it would seem
> like a bad idea to implement different semantics for f(4) and f(4.0), and
> doubles tend to be able to represent all the values of an integer.

It depends on the application, I guess. In numerical applications
nobody represents integers by doubles for various reasons (efficiency,
accuracy, etc). I understand this can be feasible for a number of
applications that hardly deal with numerics or if they do then only with
floating point numerics. And therefore they don't care if int becomes
float.

> > I don't know what is "CLOS-style multimethod.." (and therefore what
> > follows may be irrelevant)
> 
> Maybe. See http://www.sff.net/people/neelk/open-source/Multimethod.py for a
> Python-oriented take.

Thanks for the reference.

> > but I don't see how your suggestion would solve
> > the problem in hand. That is, if a wrapped class A defines methods
> >
> >  .foo(int)
> >  .foo(float)
> >
> > and a class B has both __int__ and __float__ methods, then in Python
> >
> >  A().foo(B())
> >
> > how can you tell which method, __int__ or __float__, should be called
> when
> > doing conversion? E.g. take B=int and then take B=float.
> 
> I assume you mean the cases where B() returns int or float. 

No. I mean that B() is an instance of the class B that represents either
int or float, or can be transformed to one. Take, for example, B is
Rational, or an arbitrary precision number.

> That's easy: Python int would correspond most-closely to C++ int, and
> Python float would correspond most-closely to C++ double.

I agree. But BPL does not seem to agree with that ;-)

> > In order to do it properly, at some point the responsible mechanism in
> BPL
> > should check whether an instance B() has more an "int nature" or more a
> > "float nature". This is easy, in principle, if B is int or float (or
> > a subclass of int or float) but not obvious at all if B is, for example,
> > str
> 
> neither the built-in function str() nor built-in strings have __int__ or
> __float__ methods, so in that case there would be no match at all.

And yet

>>> float("2.3")
2.2999999999999998
>>> int("2")
2

so I didn't look in to details of how these numbers are constructed.
str was a bad example. Forget it.

> > or an user defined class that defines both __int__ and __float__
> > methods.
> 
> In that case I would consider the call ambiguous.

Which is exactly the case for Python int and float:

>>> int.__int__ 
<slot wrapper '__int__' of 'int' objects>
>>> int.__float__ 
<slot wrapper '__float__' of 'int' objects>
>>> float.__float__ 
<slot wrapper '__float__' of 'float' objects>
>>> float.__int__ 
<slot wrapper '__int__' of 'float' objects>

> > Currently, I can workaround this by defining a single method
> >
> >   .foo(PyObject*)
> >
> > and explicitly checking whether an object is int or float and doing
> > the appropiate conversion in that method. This approach works fine with
> > methods with int/float arguments and I am happy with that.
> 
> I'm not, though ;-)

Good.

> > BUT, there is a real problem with constructors. Namely, one cannot define
> > an additional constructor (that is needed for being more explicit with
> > conversions for the same reasons as discussed above for methods)
> >
> >   A(PyObject*)
> >
> > without introducing a lightweighted wrapper to a library class A.
> 
> Well, of course there is a way, but it's not in the library's public
> interface.

And that way might be far over my C++ head ;-)

> > And that approach is unacceptable because it would mean that all methods
> > (that I would like to use from Python) of A, must be re-wrapped as
> > well.
> 
> No, I don't think so. You only need to wrap them once in
> class_<A,A_wrapper>. It's only constructors which need to be duplicated in
> A_wrapper.

I tried that, but it didn't work out. I'll try to sketch the real
situation from my memory: the GiNaC library contains the following class
tree (only partially exposed here, see
  http://www.ginac.de/reference/hierarchy.html
for a complete tree):

class basic
  class symbol(basic)
  class numeric(basic)
  class expairseq(basic)
    class add(expairseq)
  ..

class ex /* this is an holder of basic's pointer and used to
            pass different basic instances to various methods
            as well as to return the results of calculations
          */

In the BPL based wrapper I did

  boost::python::module m("_ginac");
  m
    .add(
    	 boost::python::class_<GiNaC::ex>("ex")
	 .def_init(boost::python::args<const GiNaC::ex &>())
	 .def_init(boost::python::args<const GiNaC::basic &>())
         /* snip number of methods */
    )
    .add(boost::python::class_<GiNaC::basic>("_basic")
	 .def_init(boost::python::args<const GiNaC::basic &>())
         /* snip number of methods */
   );
   m
    .add(boost::python::class_<GiNaC::numeric,
	 boost::python::bases<GiNaC::basic> >("numeric")
	 .def_init(boost::python::args<const numeric &>())
         /* Wished to have
          .def_init(boost::python::args<int>())
          .def_init(boost::python::args<double>())
         */
         /* snip methods */
   )

It is not clear to me how to include

  class numeric_wrapper: public GiNaC::numeric {
    numeric_wrapper(PyObject*);
  }

into this tree. If I remember correctly then

    .add(boost::python::class_<GiNaC::numeric,
         boost::python::bases<GiNaC::basic, numeric_wrapper> >("numeric")
         .def_init(boost::python::args<const numeric &>())
         ...

didn't work because the constructor of numeric_wrapper was never called -
BPL seemed to forget the constructors of the parent classes (or again
choosed the first found match).

If the above still does not make sense, I'll try to produce a working
example of the above demonstrating my situation.

> > In my particular case the number of relevant methods can be more
> > than 50 and therefore this lightweighted wrapper would get quite
> > "heavy".
> 
> I don't understand that part.

I meant that I ended up with implementing methods like

  basic_add_basic
  basic_add_pyobj
  basic_add_ex
  basic_add_numeric
  ex_add_basic
  ex_add_pyobj
  ex_add_ex
  ex_add_numeric
  numeric_add_basic    /* Though numeric is derived from basic, its
  numeric_add_numeric     arithmetics is done in a different route */
  numeric_add_ex        
  numeric_add_py

Similarly for mul,div,sub,pow,etc.

And then I noticed that the size of the wrapper was becoming close to the
size of the library itself, which stopped me and made me doubt if
this approach is a good one.

> > Now, the question is whether it is possible to introduce additional
> > constructors to library classes without deriving a new class for that?
< snip >
> > What do you think? Would the above be possible in principle?
> 
> In principle. my_A_init has to build and install a Holder; it can't just
> return a new A copy... at least, that's the way the library works now.

Thanks for the hint. I'll try to figure out that if nothing else works.

still-not-convinced-that-bpl-cannot-wrap-real-libraries'ly yours

Pearu