Python scripted interface #621

vovythevov · 2016-11-07T21:44:15Z

This branch aims to add nice-to-have python methods for scripted modules.
Those methods were typically the kind of method I found myself copy/pasting all over my scripted modules and I think they could be beneficial to the community.

vovythevov · 2016-11-07T21:44:29Z

@jcfr: Would you mind reviewing ?

jcfr

Thanks for improving util module and adding test.

Very minor nitpick: After removing the extra spaces, LGTM

jcfr · 2016-11-18T07:16:39Z

Applications/SlicerApp/Testing/Python/UtilTest.py

+ background=backgroundNode,
+ foreground=foregroundNode,
+ label=labelmapNode,
+ foregroundOpacity= 0.5,


extra space

jcfr · 2016-11-18T07:16:53Z

Applications/SlicerApp/Testing/Python/UtilTest.py

+ foreground=foregroundNode,
+ label=labelmapNode,
+ foregroundOpacity= 0.5,
+ labelOpacity= 0.1


extra space

jcfr · 2016-11-18T07:19:25Z

Applications/SlicerApp/Testing/Python/UtilTest.py

+ background=otherBackgroundNode.GetID(),
+ foreground=otherForegroundNode.GetID(),
+ label=otherLabelmapNode.GetID(),
+ foregroundOpacity= 0.0,


jcfr · 2016-11-18T07:22:59Z

Applications/SlicerApp/Testing/Python/CMakeLists.txt

@@ -292,6 +292,7 @@ if(Slicer_USE_QtTesting AND Slicer_USE_PYTHONQT)
 slicer_add_python_unittest(SCRIPT Charting.py)


in commit message, devellopers -> developers

jcfr · 2016-11-18T07:30:09Z

Base/Python/slicer/util.py

+ super(QtWidgetMixin, self).__init__()
+ self.Widget = None
+
+ def loadUI(self, path, fail_silently=False):


Convert comment to python docstring

# Attempts to load and return a ui file that the given path points to. # This method will raise an AssertionError if the ui file cannot be # loaded and fail_silently is False (default)

->

Load UI file ``path`` and return the corresponding widget. Raises a ``RuntimeError`` exception if the UI file is not found or if no widget was instantiated.

jcfr · 2016-11-18T07:30:59Z

Base/Python/slicer/util.py

+ loader = qt.QUiLoader()
+ widget = loader.load(qfile)
+ if not fail_silently and not widget:
+ errorMessage = "Could not load UI file: " + str(path) + "\n\n"


I suggest to keep the method simple and always throw a RuntimeError

jcfr · 2016-11-18T07:31:57Z

Base/Python/slicer/util.py

+ raise AssertionError(errorMessage)
+ return widget
+
+ def get(self, name):


convert to python docstring

jcfr · 2016-11-18T07:32:11Z

Base/Python/slicer/util.py

+
+ def get(self, name):
+ # Convenience method to access a widget from the self.Widget attribute.
+ # This method will throw an AssertionError exception if the widget with the


AssertionError -> RuntimeError

jcfr · 2016-11-18T07:33:28Z

Base/Python/slicer/util.py

+class QtWidgetMixin(object):
+ def __init__(self):
+ super(QtWidgetMixin, self).__init__()
+ self.Widget = None


This is not set anywhere.

Do you mean self.Widget ? This is used in the get() method. But yes, it is left to the user to be set.
(See my comment about the get() method).

jcfr · 2016-11-18T07:33:56Z

Base/Python/slicer/util.py

+ widget = loader.load(qfile)
+ if not fail_silently and not widget:
+ errorMessage = "Could not load UI file: " + str(path) + "\n\n"
+ raise AssertionError(errorMessage)


Should this be added self.Widget = widget ?

The idea is that the user can decide what is in self.Widget since this is what is access in the get() method.
Most likely, all the users would do something like this:

def MyClass(QtWidgetMixin): def __init__(self): super(MyClass, self).__init__() self.Wigdet = self.loadUI('/my_path/myWigdet.ui') myWidget = self.get('WidgetName')

But I could also see something like this:

def MyClass(QtWidgetMixin): def __init__(self): super(MyClass, self).__init__() firstPart = self.loadUI('/my_path/firstPart.ui') secondPart = self.loadUI('/my_path/secondPart.ui') self.Wigdet = qt.QWidget() layout = qt.QVBoxLayout() layout.addWidget(firstPartWidget) layout.addWidget(secondPartWidget) self.Widget.setLayout(layout) w1 = self.get('WidgetName_FromFirstPart') w2 = self.get('WidgetName_FromSecondPart')

In that case, I would recommend to pass the widget as a parameter to get:

def MyClass(QtWidgetMixin): def __init__(self): super(MyClass, self).__init__() firstPart = self.loadUI('/my_path/firstPart.ui') secondPart = self.loadUI('/my_path/secondPart.ui') self.Wigdet = qt.QWidget() layout = qt.QVBoxLayout() layout.addWidget(firstPartWidget) layout.addWidget(secondPartWidget) self.Widget.setLayout(layout) w1 = self.get('WidgetName_FromFirstPart', self.Widget) w2 = self.get('WidgetName_FromSecondPart', self.Widget)

By default, you could also automatically use self.Widget:

def MyClass(QtWidgetMixin): def __init__(self): super(MyClass, self).__init__() self.loadUI('/my_path/TopLevelWidget.ui') w = self.get('WidgetName_From_TopLevelWidget')

jcfr · 2016-11-18T07:36:32Z

Applications/SlicerApp/Testing/Python/QtMixinTest.py

+ self.test_get()
+
+ def test_loadUI(self):
+ mixinWidget = QtMixinTestWidget(auto_setup=False)


Since QtMixinTestWidget is never instantiated with auto_setup=True, would it make sense to simplify the code and remove this option ?

Good catch :)

jcfr · 2016-11-18T16:05:22Z

For the name, what do you think of QtUILoaderMixin ? Or something similar

Also add testing for those methods.

LoadUI() allows to create a widget from a .ui file and findChild() allows quick access to a particular widget by name.

vovythevov · 2016-11-21T21:05:20Z

@jcfr: I pushed an updated version if you want to take a look.

jcfr · 2016-12-08T20:51:43Z

LGTM. Please integrate. 👍

vovythevov · 2016-12-08T21:11:41Z

Integrated in r25595 and r25596

jcfr · 2016-12-09T15:40:18Z

@vovythevov The test is failing on the linux factory, do you think you could have a look: http:https://slicer.cdash.org/testDetails.php?test=7737225&build=938646

Thanks

vovythevov · 2016-12-16T16:59:26Z

@jcfr: I cannot reproduce on my UNIX machine and it seems that the isn't failing on UNIX either (although it's still failing on MAC).

jcfr · 2016-12-16T17:13:29Z

Thanks for looking into this. Let's then wait until we transition to OpenGL2 VTKk backend ...

vovythevov force-pushed the PythonScriptedInterface branch from bd57d3b to 7ffa833 Compare November 8, 2016 20:49

jcfr reviewed Nov 18, 2016

View reviewed changes

jcfr added feature wip labels Nov 18, 2016

vovythevov added 2 commits November 18, 2016 13:15

ENH: Add util method to set and fit slice views in python

ae23ad0

Also add testing for those methods.

ENH: Add util methods for easier programming of python widgets

0aecec3

LoadUI() allows to create a widget from a .ui file and findChild() allows quick access to a particular widget by name.

vovythevov force-pushed the PythonScriptedInterface branch from 7ffa833 to 0aecec3 Compare November 18, 2016 19:17

jcfr approved these changes Dec 8, 2016

View reviewed changes

vovythevov closed this Dec 8, 2016

vovythevov deleted the PythonScriptedInterface branch December 8, 2016 21:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python scripted interface #621

Python scripted interface #621

vovythevov commented Nov 7, 2016

vovythevov commented Nov 7, 2016

jcfr left a comment

jcfr Nov 18, 2016

jcfr Nov 18, 2016

jcfr Nov 18, 2016

jcfr Nov 18, 2016

jcfr Nov 18, 2016

jcfr Nov 18, 2016

jcfr Nov 18, 2016

jcfr Nov 18, 2016

jcfr Nov 18, 2016

vovythevov Nov 18, 2016

jcfr Nov 18, 2016

vovythevov Nov 18, 2016 •

edited

Loading

jcfr Nov 18, 2016 •

edited

Loading

jcfr Nov 18, 2016

vovythevov Nov 18, 2016

jcfr commented Nov 18, 2016

vovythevov commented Nov 21, 2016

jcfr commented Dec 8, 2016

vovythevov commented Dec 8, 2016

jcfr commented Dec 9, 2016

vovythevov commented Dec 16, 2016

jcfr commented Dec 16, 2016

		@@ -292,6 +292,7 @@ if(Slicer_USE_QtTesting AND Slicer_USE_PYTHONQT)
		slicer_add_python_unittest(SCRIPT Charting.py)

Python scripted interface #621

Python scripted interface #621

Conversation

vovythevov commented Nov 7, 2016

vovythevov commented Nov 7, 2016

jcfr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vovythevov Nov 18, 2016 • edited Loading

Choose a reason for hiding this comment

jcfr Nov 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcfr commented Nov 18, 2016

vovythevov commented Nov 21, 2016

jcfr commented Dec 8, 2016

vovythevov commented Dec 8, 2016

jcfr commented Dec 9, 2016

vovythevov commented Dec 16, 2016

jcfr commented Dec 16, 2016

vovythevov Nov 18, 2016 •

edited

Loading

jcfr Nov 18, 2016 •

edited

Loading