VTK review process documentation

@dgobbi The script seems to work quite well, I had to do a couple of little changes and bring in the option to manually add some imports in a similar way to what we do when developing C++ code. For example ElevationBandsWithGlyphs needs

# noinspection PyUnresolvedReferences
import vtkmodules.vtkRenderingOpenGL2

whilst CurvatureBandsWithGlyphs does not. Excepting for the need to include numpy stuff, the imports are almost identical. The same modules are used - just differing by imported classes. This was tested with PyCharm and on the command line using vtkpython and also python directly. Other code, for example CurvaturesAdjustEdges, also doesn’t need to import vtkRenderingOpenGL2.

Another interesting thing when using PyCharm is that

from vtkmodules.vtkInteractionWidgets import vtkCameraOrientationWidget

trigers a Cannot find reference 'vtkCameraOrientationWidget' in '__init__.py' in Windows but not in Linux.

Hi Andrew, what does “noinspection PyUnresolvedReferences” do?

My feeling is that it’s best to put “import vtkmodules.vtkRenderingOpenGL2” after importing the other rendering modules, instead of putting it at the top. The reason for this is that the other modules (vtkRenderingCore etcetera) are its dependencies, i.e. they come earlier in the topological sort.

CurvatureBandsWithGlyphs is still importing the “vtk” module and therefore it’s still pulling in everything. For vtkCameraOrientationWidget, it’s not in the release branch, only in the master.

When you format the Python code in PyCharm unused imports are removed so “noinspection PyUnresolvedReferences” prevents their removal. PyCharm by default also sorts the imports alpabetically when Reformat Code run.

It can be disabled in PyCharm: Settings/Preferences | Editor | Code Style | Python | Imports so I will redo the imports. The script sorts the from ... import ... statements alphabetically and adds the additional commented out imports at the end thus it is no big deal. So I will do that and also move from vtk.util import numpy_support to the end on the imports.

I see this is the line from vtk.util import numpy_support this explains a lot! I understand now, thanks.

I am actually using the master in both Windows and Linux.

Thanks for your comments it has really clarified a lot for me.

The module .py files import (direct VTK module) dependencies automatically, so there’s no real need for users to know the toposort of the modules they’d like to import.

I do realize this, but if the list of modules is being created by a script (as is the case here), topological ordering makes the most sense. The reason: if an exception occurs during import, this gives the shortest and most informative stack trace. This is particularly true when importing .so/.pyd modules as opposed to .py modules.

From my viewpoint I don’t like the idea of disabling the default import sorting in PyCharm because it adds another layer of complexity for new users and for users switching between different machines and operating systems. It’s years since I have used Eclipse but I suspect imports are also sorted by default. So I would like to keep default settings on IDE’s as much as possible.One less thing to check when people submit a MR.

I agree, but you’re fighting tooling at that point and anything that isn’t topo == alpha order means inserting extra newlines to keep formatters happy. The fix is to just be robust to any import order just like we should be with C++ headers because we could keep those topo-sorted too, but discovering and remembering that order is way more difficult (and even Python is hard). The best we end up doing is separating into groups like “own header”, “VTK”, “stdlib”, “extdeps”, etc.

@ben FYI, I rewrote and modernised VTK/Maintenance/WhatModulesVTK.py and renamed it VTKModulesForCxx so it is now in the VTK Examples alongside VTKImportsForPython.