[Bf-committers] Freestyle branch status report May 2012

[Tamito KAJIYAMA]

Campbell,

Thanks again for the comments.  And excuse me if my reaction sounded in any
negative sense.  I mentioned the Python API auto-generation just to anticipate
a possible future direction and expected difficulties.

I fully agree that it would be nice to follow the existing Pythonic conventions
of the Blender Python API.

So far the Freestyle Python API is the only component that was pointed out as
an issue preventing the trunk merger.  I think I am going to focus on API revisions
(by manual editing for now) for better consistency and come back to here later
to ask for code review.

Regards,

-- 
KAJIYAMA, Tamito <rd6t-kjym at asahi-net.or.jp>


-----Original Message----- 
From: Campbell Barton 
Sent: Thursday, May 24, 2012 7:43 AM 
To: bf-blender developers 
Subject: Re: [Bf-committers] Freestyle branch status report May 2012 

@Tamito, I figured auto generation wasn't really an option here so
didn't mention it - of course if it was practical it would be a nice
time saver.

We already have gone to great lengths to get more consistent API's for
blender so Its nice if additional APIs can follow.
Though I realise we could consider freestyle an external application
and not attempt consistency.
However from looking at the code I think its worth doing to remove
duplication alone.

- You  could post link to bf-python --- api docs & some example
scripts to help others get an overview of how this works

- Review existing blender python api's in blender --- bmesh_py_*.c are
good examples of a pythonic api which closely follows internal C API.

If this is near being accepted I can help out a bit with an api update
before merging.

On Thu, May 24, 2012 at 1:54 AM, Tamito KAJIYAMA
<rd6t-kjym at asahi-net.or.jp> wrote:
> Dan and Campbell,
>
> Thanks for your review of the Python part.  I am going to address all the
> suggested changes and improvements.
>
> The current naming convention of the Freestyle Python API is largely historical
> and based on the underlying C++ class hierarchy.  Previously the Python API was
> auto-generated by SWIG.  Then we rewrote the Python API by manual coding in
> order to get rid of the C++ syntax from the Python API.
>
> For what concerns an automatic generation of the Python API from the C++ code,
> there is an application requirement.  The Freestyle Python API is a wrapper of the
> underlying C++ class system, and the Python and C++ layers have been connected
> by a specific mechanism that allows you to define a Python subclass of a C++ class.
> The present API implementation heavily relies on this mechanism, and indeed
> it works very well.  Implementing a similar (automated) functionality in makerna
> would be a major task.
>
> --
> KAJIYAMA, Tamito <rd6t-kjym at asahi-net.or.jp>
>
>
> -----Original Message-----
> From: Campbell Barton
> Sent: Tuesday, May 22, 2012 6:57 AM
> To: bf-blender developers
> Subject: Re: [Bf-committers] Freestyle branch status report May 2012
>
> On Tue, May 22, 2012 at 4:05 AM, Dan Eicher <dan at trollwerks.org> wrote:
>> Quickly looking over the python part of the patch, there's lots of
>> places where you should be using PyGetSetDef for attributes instead of
>> function getters/setters -- for eg StrokeShader.name instead of
>> StrokeShader.getName().
>>
>> Don't know if makesrna can do C++ but that would make it a lot easier
>> than maintaining a bunch of hand written python bindings. Probably
>> really depends on if the generated code is compiled by C++ or not
>> because the access functions would be a breeze.
>>
>> Dan
>
> Yep, if you can use PyGetSetDef this would make a lot more sense for
> BPy_FrsMaterial_methods for eg,
> Now mathutils defines mathutils.Color too which could be used instead
> of r/g/b accessors.
>
> if the data isnt in DNA then wrapping like this is needed but think it
> could be improved.
>
> Would be better to do full codereview but noticed some other things.
>
> - suffers from api clogging we had in 2.4x - eg:
> -- StrokeVertex_setX
> -- StrokeVertex_setY
> -- StrokeVertex_setPoint(x, y)
> -- StrokeVertex_x
> -- StrokeVertex_y
> -- StrokeVertex_getPoint
> ... all have docstrings too - 12 definitions which could be replaced
> with one attribute ".point" ? and one docstring
>
> - PyTuple_SET_ITEM can be used when you create the tuple.
> - keywords are ignored in some initializers.
> - api doesnt use pep8/blender api naming conventions (lowercase +
> underscores for methods and attributes, CamelCase for classes)
> - scripts use import * - bad practice.
> - defines "__getitem__" method rather then implementing Mapping or
> Sequence protocols - PyMappingMethods / PySequenceMethods.
> - bool_from_PyBool accepts any object type and doesn't typecheck -
> could lead to confusing errors if "foo.bar = None" is equivalent to
> "foo.bar = False" and "foo.bar = 2.0" is equivalent to "foo.bar =
> True" rather then raising an error.