[vtk-developers] Improvements to the Python wrapping in VTK

Tue Mar 19 17:53:47 EDT 2013

On Tue, Mar 19, 2013 at 5:47 PM, David Gobbi <david.gobbi at gmail.com> wrote:
> On Tue, Mar 19, 2013 at 3:18 PM, Marcus D. Hanwell
> <marcus.hanwell at kitware.com> wrote:
>> On Tue, Mar 19, 2013 at 5:03 PM, Marcus D. Hanwell
>> <marcus.hanwell at kitware.com> wrote:
>>> On Tue, Mar 19, 2013 at 4:30 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>> On Tue, Mar 19, 2013 at 1:58 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>>> On Tue, Mar 19, 2013 at 1:34 PM, Marcus D. Hanwell
>>>>> <marcus.hanwell at kitware.com> wrote:
>>>>>> On Tue, Mar 19, 2013 at 9:43 AM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>>>>> On Mon, Mar 18, 2013 at 6:08 PM, Marcus D. Hanwell
>>>>>>> <marcus.hanwell at kitware.com> wrote:
>>>>>>>> On Mon, Mar 11, 2013 at 1:51 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>>>>>>> On Mon, Mar 11, 2013 at 11:37 AM, Marcus D. Hanwell
>>>>>>>>> <marcus.hanwell at kitware.com> wrote:
>>>>>>>>>> On Mon, Mar 11, 2013 at 1:34 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>>>>>>>>> On Mon, Mar 11, 2013 at 11:01 AM, Marcus D. Hanwell
>>>>>>>>>>> <marcus.hanwell at kitware.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> That sounds great - I will make that move (and agree on reducing
>>>>>>>>>>>> moves). My final question if you look at the patch is on the right way
>>>>>>>>>>>> to handle vtkType.h and friends - should they just be marked as
>>>>>>>>>>>> WRAP_SPECIAL, should we do something different with WRAP_HEADER (which
>>>>>>>>>>>> is currently going into the classes file).
>>>>>>>>>>>
>>>>>>>>>>> I want to export all python-wrappable files, including vtkType.h.  But
>>>>>>>>>>> there is no vtkCommonCore-Headers.cmake, so I don't know where to put
>>>>>>>>>>> vtkType.h except in vtkCommonCore-Classes.cmake, which is a poor fit.
>>>>>>>>>>>
>>>>>>>>>>> The main purpose of both WRAP_HEADER and WRAP_SPECIAL is
>>>>>>>>>>> just to wrap files in python that don't get wrapped in tcl or java.  The
>>>>>>>>>>> WRAP_HEADER is just for wrapping headers that aren't already in the
>>>>>>>>>>> class list.  So of course I'd like for WRAP_HEADER headers to be
>>>>>>>>>>> exported, but really I'd prefer to export all non-class headers
>>>>>>>>>>> anyway.
>>>>>>>>>>>
>>>>>>>>>>> Since I never extended the "special" wrapping to the java and tcl
>>>>>>>>>>> wrappers (and almost certainly won't) maybe a WRAP_PYTHON
>>>>>>>>>>> should replace both WRAP_HEADER and WRAP_SPECIAL.
>>>>>>>>>>>
>>>>>>>>>> I feel like if WRAP_SPECIAL is simply ignored by the other wrapped
>>>>>>>>>> languages keeping that name seems reasonable. If we want to extend
>>>>>>>>>> either one (or another language in the future) then we can use
>>>>>>>>>> WRAP_SPECIAL as an extended set of files that your wrapper must
>>>>>>>>>> support and get rid of WRAP_HEADER entirely. It seems to me like they
>>>>>>>>>> belong in the -Classes file, but perhaps that name could be a little
>>>>>>>>>> more general (-Headers as we only really care about headers for
>>>>>>>>>> wrapping?)
>>>>>>>>>
>>>>>>>>> I'd also prefer -Headers over -Classes.  A complete list of
>>>>>>>>> every header file that is part of the install, along with a list
>>>>>>>>> of properties.
>>>>>>>>>
>>>>>>>> I am just working on cleaning this topic up. I noticed that since
>>>>>>>> moving the vtkWrappingPython module three Python tests are now
>>>>>>>> failing,
>>>>>>>>
>>>>>>>> http://open.cdash.org/viewTest.php?onlyfailed&buildid=2842500
>>>>>>>>
>>>>>>>> It seems that there is some hardwired logic I am not aware of - any
>>>>>>>> ideas? This is the test string, templates and variants so it all fits
>>>>>>>> that the special code is being missed somehow.
>>>>>>>
>>>>>>> These failures happen because some classes appear in both the
>>>>>>> SRCS and HDRS, and that confuses the vtkWrapHierarchy  tool.
>>>>>>> Specifically, vtkTypedArray, vtkSparseArray, vtkDenseArray appear
>>>>>>> as both .h and as .txx files.  The Hierarchy.txt files end up with two
>>>>>>> entries for each, one marked as WRAP_EXLUDE;WRAP_SPECIAL
>>>>>>> (which is correct) with a duplicate marked as just WRAP_EXCLUDE.
>>>>>>
>>>>>> I am just getting my head around this now, I guess this is because the
>>>>>> txx need to be installed, but why not list the txx in the source (as
>>>>>> they are closer to the source) and add some code to deal with txx
>>>>>> files correctly? This doesn't seem to affect the -Headers file either,
>>>>>> which only shows one occurrence of each of these classes. So if I fix
>>>>>> the duplication then wrap hierarchy should be happy? It would be good
>>>>>> to get all of the wrapping tools looking at the same information, but
>>>>>> I am not sure I will have the time necessary to carry this through for
>>>>>> everything.
>>>>>
>>>>> Yes, hierarchy will be happy if:
>>>>> 1) the duplication is removed
>>>>> 2) whichever file appears in SRCS also appears in the WRAP_SPECIAL list.
>>>>>
>>>>> So right now only vtkDenseArray.h appears in WRAP_SPECIAL which
>>>>> is why the hierarchy listing that corresponded to vtkDenseArray.txx lacked
>>>>> the WRAP_SPECIAL property and wasn't properly wrapped.
>>>>>
>>>>> Having the .txx in SRCS and the .h unlisted makes sense to me for these
>>>>> classes.
>>>>
>>>> But I just thought of something.  I have some vague memories of some classes
>>>> that had .h, .txx, and .cxx (all three).  I can't remember if it was VTK or ITK.
>>>> Really this should never happen because VTK is supposed to have one class
>>>> per header, and that class is either templated or it isn't.
>>>>
>>> I haven't come across that, and like you say it seems like it should
>>> not be possible. I pushed an update which (locally at least) builds
>>> with Tcl turned on and the templates test in Python is working.
>>> Effectively I added some code to treat txx a little more like a source
>>> file, with the caveat that we would install the txx (not the case with
>>> cxx). I will wait on the dashboards, and build Java while I am
>>> waiting.
>>>
>>> Feedback welcome, I think this is getting pretty close and would
>>> appreciate any further feedback you have on the topic.
>>>
>> As far as I can tell, on Linux at least, this is fine with all wrapped
>> languages (Python, Tcl, Java) and instantiators turned on. To be
>> exported you need to place the .cxx, .txx, .mm or .h in the list of
>> sources for the module, we will then attempt to find the matching
>> header file. It might be cleaner to directly list all public headers
>> that are candidates for wrappnig/part of our public API but that would
>> also take quite a bit of time. I feel like all of the properties
>> should be on the headers too, but this again would take some time and
>> only becomes reliable if all of the wrapping tools are consulting the
>> same source of information.
>
> If the .cxx/.txx is already listed it would be redundant to have the
> corresponding .h listed, too.  I have mixed feelings about applying
> the properties to the headers.  On the "pro" side, the header is the
> interface and the property reflects the interface more than anything
> else.  On the "con" side, listing the .cxx in SRCS but then applying
> properties to the .h files seems confusing.
>
Yeah, I was more thinking out loud, but was it is in the topic is all
I intended to do in the short term (unless you spot problems with it).
I think the code to make the transformation from source to header is
fairly robust, and I see the tests all passed on the dashboard
machines too (along with Tcl, Java and instantiators locally on
Linux).

Marcus