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

[David Gobbi]

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.

 - David