View Issue Details

IDProjectCategoryView StatusLast Update
0004462Slicer4Core: Scripting (Wrapping, Python)public2018-01-09 04:47
ReporterpinterAssigned Tojcfr 
PriorityhighSeverityblockReproducibilityalways
Status feedbackResolutionreopened 
Product VersionSlicer 4.9.0 
Target VersionSlicer 4.9.0Fixed in VersionSlicer 4.9.0 
Summary0004462: Python wrapping of VTK classes in extensions does not work properly
Description

The VTK classes don’t seem to be recognized as their proper class, similarly to the vtkSegmentationCore classes without importing the python module, but this time even when I import it specifically the type is wrong (which means that their methods cannot be called):

import vtkSlicerDicomRtImportExportModuleLogicPython
slicer.modules.dicomrtimportexport.logic() # DicomRtImportExport logic is only recognized as its base class
(SlicerBaseLogicPython.vtkSlicerModuleLogic)0000019BF9E1DB88
slicer.modules.volumes.logic() # Volumes logic is recognized correctly
(vtkSlicerVolumesModuleLogicPython.vtkSlicerVolumesLogic)0000019BF9E1DBE8

Steps To Reproduce

In SlicerRT:

TagsNo tags attached.

Relationships

has duplicate 0004448 closedjcfr Python wrapping of MRML classes not working with VTK8 build 

Activities

Davide

Davide

2017-10-31 09:02

developer   ~0015369

Hi Jc, do you have any news regarding this? do we have to update the extensions in some ways or it will be handled by the wrapping macro?

pinter

pinter

2017-10-31 10:58

developer   ~0015370

I'm happy to look into it if I know where to look.
Jc if you're too busy to work on this in the next few days, please send me your ideas and I'll start from those. Thanks!

Davide

Davide

2017-11-01 05:22

developer   ~0015371

I guess should be related to this: https://www.slicer.org/wiki/Documentation/Labs/Qt5-and-VTK8#VTK8:_Use_hierarchy_files_for_VTK_Python_wrapping
However, I have no idea how to proceed.

pinter

pinter

2017-11-01 09:29

developer   ~0015373

Thanks, Davide, I'll take a closer look at these hierarchy files. When I read this page, it seems I conveniently skipped through this section and thought the type macros should cut it...

jcfr

jcfr

2017-11-01 10:33

administrator   ~0015374

Hi,

Hierarchy files should also be generated within extensions. See [1]

I don't know what is happening yet, my understanding was that after the fix of Andras (see r26418 [2]) we were in good shape.

[1] https://github.com/Slicer/Slicer/blob/e77630416ec79c7cb6a8145c6286b39a25ebaec1/CMake/vtkMacroKitPythonWrap.cmake#L113-L215

[2] See viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=26418

pinter

pinter

2017-11-01 11:00

developer   ~0015375

The hierarchy fines are indeed generated. In Slicer-build I found the SlicerRT hierarchy files, for example vtkSlicerDicomRtImportExportModuleLogicHierarchy.txt. It is very big and contains more than 4000 classes, but the classes that I mention in the issue description are there as well:

vtkSlicerDicomRtImportExportModuleLogic : vtkSlicerModuleLogic ; vtkSlicerDicomRtImportExportModuleLogic.h ; vtkSlicerDicomRtImportExportModuleLogic
vtkSlicerDicomRtImportExportModuleLogic::Superclass = vtkSlicerModuleLogic ; vtkSlicerDicomRtImportExportModuleLogic.h ; vtkSlicerDicomRtImportExportModuleLogic

The class however is still not recognized. I'm on a 5 days old revision, and the same thing happens as what I describe above.

Davide

Davide

2017-11-01 11:09

developer   ~0015376

Hi jc and Csaba, thanks for checking it.

I tested again with last version of Slicer (master at e77630416ec79c7cb6a8145c6286b39a25ebaec1) on my local build (ubuntu 17.10).
When I build the extension I can actually find the hierarchy files, but they are in the Slicer folder build (Slicer-SuperBuild-Debug/Slicer-build).
However if I try in the python console to call a MRML class of my extension, I get still the same error:

import slicer
slicer.vtkMRMLAstroVolumeNode()

I get the error:
AttributeError: 'module' object has no attribute 'vtkMRMLAstroVolumeNode'

Davide

Davide

2017-11-01 11:10

developer   ~0015377

ah you found that as well (:

pinter

pinter

2017-11-01 11:11

developer   ~0015378

Yep, same thing. What we need to figure out I guess is why those seemingly correct hierarchy files are ignored.

Davide

Davide

2017-11-14 05:11

developer   ~0015405

Hi, I just come back from a conference, so I didn't have the time to check this further up to now. I just wanted to check again if you have any news.

pinter

pinter

2017-11-16 10:12

developer   ~0015407

Unfortunately no news, I'm waiting for some pointers from Jc to have an idea where to start.

Davide

Davide

2017-11-16 11:11

developer   ~0015408

Ok, thanks for letting me know. I have to say that I also have no clue where to start (:

jcfr

jcfr

2017-11-20 11:36

administrator   ~0015414

To follow up on this, I was able to reproduce the problem and I am currently investigating.

pinter

pinter

2017-11-20 11:56

developer   ~0015415

Excellent, thank you Jc! Let me know if I can do something to help.

jcfr

jcfr

2017-11-20 19:15

administrator   ~0015417

This should be fixed in https://github.com/Slicer/Slicer/pull/845

If you don't have any comment on the PR, I will integrate later tonight.

pinter

pinter

2017-11-20 21:43

developer   ~0015418

I can't say I can completely follow what the problem was exactly, but thank you for the fix :) Please integrate. I'll test it on Windows and Mac tomorrow.

jcfr

jcfr

2017-11-20 22:28

administrator   ~0015419

Fixed in r26649
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=26649

I can't say I can completely follow what the problem was exactly

To properly wrap classes, the "VTK wrapper" needs information associated with all types used in the public interface of classes that should be wrapped.

Such information are stored in "Hierarchy" files.

The commit r26649 makes sure Slicer types are all exported so that they can be used within the extension build system.

For more details, see https://www.vtk.org/Wiki/VTK/WrapHierarchy

Davide

Davide

2017-11-21 05:21

developer   ~0015420

Last edited: 2017-11-21 05:37

View 3 revisions

@Jcfr great!!! I tested locally (ubuntu) and it works for Logic, MRML and Widget classes. I have only one issue, in SlicerAstro I have also a display manager
https://github.com/Punzo/SlicerAstro/tree/master/AstroVolume/MRMLDM

and at compilation now I get this error during the wrapping:
[ 61%] Python Wrapping - generating vtkMRMLAstroTwoDAxesDisplayableManagerPython.cxx
Couldn't locate header file vtkMRMLAbstractDisplayableManager.h
AstroVolume/MRMLDM/CMakeFiles/vtkSlicerAstroVolumeModuleMRMLDisplayableManagerPythonD.dir/build.make:73: recipe for target 'AstroVolume/MRMLDM/vtkMRMLAstroTwoDAxesDisplayableManagerPython.cxx' failed
make[2]: [AstroVolume/MRMLDM/vtkMRMLAstroTwoDAxesDisplayableManagerPython.cxx] Error 1
make[2]:
Deleting file 'AstroVolume/MRMLDM/vtkMRMLAstroTwoDAxesDisplayableManagerPython.cxx'
CMakeFiles/Makefile2:1956: recipe for target 'AstroVolume/MRMLDM/CMakeFiles/vtkSlicerAstroVolumeModuleMRMLDisplayableManagerPythonD.dir/all' failed
make[1]: [AstroVolume/MRMLDM/CMakeFiles/vtkSlicerAstroVolumeModuleMRMLDisplayableManagerPythonD.dir/all] Error 2
Makefile:162: recipe for target 'all' failed
make:
[all] Error 2

Shall I update https://github.com/Punzo/SlicerAstro/blob/master/AstroVolume/MRMLDM/CMakeLists.txt or include in some way the header file or something else?

Davide

Davide

2017-11-21 05:59

developer   ~0015421

In addition, for SlicerMacroPythonWrapModuleVTKLibrary
(see examples:
https://github.com/Punzo/SlicerAstro/blob/master/vtkOpenGLFilters/CMakeLists.txt
https://github.com/Punzo/SlicerAstro/blob/master/vtkFits/CMakeLists.txt )

the Hierarchy files are generated, but again I think they are ignored, because the classes are not accessible by the python console.

Davide

Davide

2017-11-21 08:19

developer   ~0015422

I just tested that the wrapping of vtkFits and vtkOpenGLFilters was not working in Qt4/VTK7 either. So maybe the issue in these two cases is my cmake files.

Instead the wrapping of MRMLDM classes in Qt4/VTK7 was working.

jcfr

jcfr

2017-11-21 09:59

administrator   ~0015423

Hi Davide,

Since the change only affects VTK8 or VTK9, it explains why you didn't see any changes with VTK7.

I am now building SlicerAstro.

Davide

Davide

2017-11-21 10:07

developer   ~0015424

Yeah I tested to be sure that was the VTK7->VTK8/9 the issue and not some bugs in my cmakes (:. Thanks a lot for investingating further this issue!!!

jcfr

jcfr

2017-11-21 14:25

administrator   ~0015428

Hi Davide,

I didn't have any issues compiling against Slicer with Qt5 or Qt4.

Please, note that library like vtkFits have to be imported explicitly and will not be added to the slicer namespace. We would need to update the infrastructure to support it.

Here is the summary:

SlicerAstro + Qt4 + VTK7:

(1) Start Slicer using ./Slicer --additional-module-path /home/jcfr/Projects/SlicerAstro-Release-Qt4/inner-build

(2) Then open python console:

>>> import vtkFitsPython as vtkFits
>>> vtkFits.vtkFITSReader()
(vtkCommonCorePython.vtkFITSReader)0x7f89b4425738
>>> dir(vtkFits)
['__doc__', '__file__', '__name__', '__package__', 'vtkFITSReader', 'vtkFITSWriter']

SlicerAstro + Qt5 + VTK9:

(1) Start Slicer using ./Slicer --additional-module-path /home/jcfr/Projects/SlicerAstro-Release/inner-build/

(2) Then open python console:

>>> import vtkFitsPython as vtkFits
>>> vtkFits.vtkFITSReader()
(vtkFitsPython.vtkFITSReader)0x7f8940060668
>>> dir(vtkFits)
['__doc__', '__file__', '__name__', '__package__', 'vtkFITSReader', 'vtkFITSWriter']
>>> dir(vtkFits)
Davide

Davide

2017-11-22 06:28

developer   ~0015429

Hi Jc,

thanks for looking into it.

1) MRMLDM: I did two clean builds: Qt5+VTK8 and Qt5+VTK9. For the Qt5+VTK9 everything worked perfectly.
The wrapping issue for the MRMLDM class shows up only for the VTK8 build. I guess doesn't make any sense to fix the issue for VTK8, if the next binaries will have VTK9.

2) vtkFits wrapping: I am an idiot, I thought they were magically included in the slicer import (: The "manual" import is completly fine. Thanks again for checking.

pinter

pinter

2017-11-23 10:49

developer   ~0015437

I can confirm it works now, and Davide's issue seems to be solved as well. Closing the issue.
Thanks a lot, Jc!

pinter

pinter

2017-11-28 16:02

developer   ~0015447

Reopening for one last question. I found one more difference in python wrapping between VTK7 and 8. Not even sure if this was expected to work with VTK7, but it did. The source file
https://github.com/SlicerRt/SlicerRT/blob/master/SlicerRtCommon/SlicerRtCommon.h
although it is not a VTK class, was wrapped correctly with VTK7, see SlicerRtCommonPython_VTK7.cxx, but is not wrapped with VTK8, see SlicerRtCommonPython_VTK8.cxx

Jc can you comment this please? If this is something to fix, or something that was never supposed to work, etc. Thanks!

pinter

pinter

2017-11-28 16:03

developer  

SlicerRtCommonPython_VTK7.cxx (14,566 bytes)
SlicerRtCommonPython_VTK8.cxx (416 bytes)

Issue History

Date Modified Username Field Change
2017-10-19 11:38 pinter New Issue
2017-10-19 11:38 pinter Status new => assigned
2017-10-19 11:38 pinter Assigned To => jcfr
2017-10-31 06:07 Davide Relationship added has duplicate 0004448
2017-10-31 09:02 Davide Note Added: 0015369
2017-10-31 10:58 pinter Note Added: 0015370
2017-11-01 05:22 Davide Note Added: 0015371
2017-11-01 09:29 pinter Note Added: 0015373
2017-11-01 10:33 jcfr Note Added: 0015374
2017-11-01 11:00 pinter Note Added: 0015375
2017-11-01 11:09 Davide Note Added: 0015376
2017-11-01 11:10 Davide Note Added: 0015377
2017-11-01 11:11 pinter Note Added: 0015378
2017-11-14 05:11 Davide Note Added: 0015405
2017-11-16 10:12 pinter Note Added: 0015407
2017-11-16 11:11 Davide Note Added: 0015408
2017-11-20 11:36 jcfr Note Added: 0015414
2017-11-20 11:56 pinter Note Added: 0015415
2017-11-20 19:15 jcfr Note Added: 0015417
2017-11-20 21:43 pinter Note Added: 0015418
2017-11-20 22:28 jcfr Note Added: 0015419
2017-11-20 22:28 jcfr Status assigned => resolved
2017-11-20 22:28 jcfr Resolution open => fixed
2017-11-20 22:28 jcfr Fixed in Version => Slicer 4.9.0
2017-11-21 05:21 Davide Note Added: 0015420
2017-11-21 05:29 Davide Note Edited: 0015420 View Revisions
2017-11-21 05:37 Davide Note Edited: 0015420 View Revisions
2017-11-21 05:59 Davide Note Added: 0015421
2017-11-21 08:19 Davide Note Added: 0015422
2017-11-21 09:59 jcfr Note Added: 0015423
2017-11-21 10:07 Davide Note Added: 0015424
2017-11-21 14:25 jcfr Note Added: 0015428
2017-11-22 06:28 Davide Note Added: 0015429
2017-11-23 10:49 pinter Status resolved => closed
2017-11-23 10:49 pinter Note Added: 0015437
2017-11-28 16:02 pinter Status closed => feedback
2017-11-28 16:02 pinter Resolution fixed => reopened
2017-11-28 16:02 pinter Note Added: 0015447
2017-11-28 16:03 pinter File Added: SlicerRtCommonPython_VTK7.cxx
2017-11-28 16:03 pinter File Added: SlicerRtCommonPython_VTK8.cxx