Potential issues with upcoming changes in VTK

[berkgeveci]

Dear Vedo developers,

I am one of the VTK developers. We are making some changes to the VTK Python wrappers which might be disruptive to some of the Vedo functionality. Please see this discussion:
pyvista/pyvista#5470

I have run a few of the examples and did not observe any issues. However, more testing is necessary. If you can point me to a resource on running your test suite, I would be happy to try it out. If you can also point me to any Vedo classes that inherit from VTK, I can check them. There should not be any issues outside those classes.

PS: Big fan of Vedo.

[marcomusy]

Thanks Berk! I've seen your work in the vtk discourse and it looks really cool!
I'm quite confident that vedo should not suffer too much from the type of changes you are introducing.
Indeed there's a number of classes that inherit from vtk directly:

 > grep "class " vedo/*.py | grep "vtk\." 
vedo/addons.py:class Flagpost(vtk.vtkFlagpoleLabel):
vedo/addons.py:class LegendBox(vtk.vtkLegendBoxActor):
vedo/addons.py:class SplineTool(vtk.vtkContourWidget):
vedo/addons.py:class SliderWidget(vtk.vtkSliderWidget):
vedo/addons.py:class PlaneCutter(vtk.vtkPlaneWidget):
vedo/addons.py:class BoxCutter(vtk.vtkBoxWidget):
vedo/addons.py:class SphereCutter(vtk.vtkSphereWidget):
vedo/addons.py:class RendererFrame(vtk.vtkActor2D):
vedo/addons.py:class Icon(vtk.vtkOrientationMarkerWidget):
vedo/addons.py:class Ruler2D(vtk.vtkAxisActor2D):
vedo/assembly.py:class Group(vtk.vtkPropAssembly):
vedo/assembly.py:class Assembly(vtk.vtkAssembly):
vedo/interactor_modes.py:class MousePan(vtk.vtkInteractorStyleUser):
vedo/visual.py:class Actor2D(vtk.vtkActor2D):

but if I understand these are not the ones you are focusing on (?).
You can run the tests with

cd tests/common
./run_all.sh

I can also point to a few examples which you may run to check if anything bad happens with the above classes:

python examples/basic/mirror.py
python examples/basic/legendbox.py
python examples/basic/spline_tool.py
python examples/basic/sliders_hsv.py
python examples/volumetric/slice_plane3.py
python examples/other/icon.py
python examples/pyplot/plot_pip.py
python examples/volumetric/read_volume1.p

Thanks!

[berkgeveci]

Thanks @marcomusy . I found a few minor issues. One is actually due to a bug in the new wrappers that we are going to look into. The other one is in the class PlaneCutter. The new wrappers processes all of the keyword arguments in the constructor before the subclasses __init__() is called (I guess this is designed such that the type is initialized in C before any Python code is called). This includes initializing the normal, which does this:

     @normal.setter
     def normal(self, value):
         """Set the normal of the plane."""
        self.widget.SetNormal(value)

Because __init__() has not been called yet, there is no self.widget. I fixed it by doing this:

     @normal.setter
     def normal(self, value):
         """Set the normal of the plane."""
         if hasattr(self, "widget"):
             self.widget.SetNormal(value)

I also ran all of the tests and they all pass. However, I am not sure that they do anything with the plotting classes, which seems to be where the issues have come up so far.

[marcomusy]

Thanks for testing this! I'm puzzled as to why the PlaneCutter object here would not have widget as a member as that seems unrelated to vtk (?)

However, I am not sure that they do anything with the plotting classes, which seems to be where the issues have come up so far.

that is true.. though I guess the bunch of examples above should serve as a good test..

[berkgeveci]

Thanks for testing this! I'm puzzled as to why the PlaneCutter object here would not have widget as a member as that seems unrelated to vtk (?)

I figured it out. It's a bug on the wrapper side. We have been adding the keyword argument parsing code to the equivalent of __new__() (tp_new here https://docs.python.org/3/c-api/typeobj.html) as opposed to __init__(). The tp_new method is always called (without the need of chaining) before __init__() so we were initializing the widget property before it was initialized. Moving this code would fix this issue and prevent wrapper initialization of any properties unless a subclass explicitly adds it to super().__init__(). We should have a fix soon.

[berkgeveci]

I think that we are good. With the latest changes to the wrapping code, I was able to run a bunch of examples without any changes to vedo.

[marcomusy]

Fantastic! Thanks Berk for thinking of vedo, and for this super cool development in the wrappers.

PS: if can "vote" i actually like the original >> more than the| symbol for creating filter pipelines :)