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

[Marcus D. Hanwell]

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.

Thanks,

Marcus