silx icon indicating copy to clipboard operation
silx copied to clipboard

[ROI] error when adding a ROI to a RoiManager before setting geometry

Open PiRK opened this issue 6 years ago • 2 comments

At the moment, we can only add a ROI once the geometry is set.

I'm not sure the error itself is a bug. Adding a ROI without geometry is maybe nonsensical. But it makes me wonder if the geometry should not be a mandatory __init__ parameter.

Or maybe a more explicit error message would be enough.

Traceback (most recent call last):
File "debug_roi_color.py", line 30, in roiManager.addRoi(roi)
File "C:\Users\Pierre\AppData\Roaming\Python\Python36\site-packages\silx\gui\plot\tools\roi.py", line 294, in addRoi
         roi.setParent(self)
File "C:\Users\Pierre\AppData\Roaming\Python\Python36\site-packages\silx\gui\plot\items\roi.py", line 94, in setParent
        self._createPlotItems()
File "C:\Users\Pierre\AppData\Roaming\Python\Python36\site-packages\silx\gui\plot\items\roi.py", line 320, in _createPlotItems
        self._labelItem = self._createLabelItem()
File "C:\Users\Pierre\AppData\Roaming\Python\Python36\site-packages\silx\gui\plot\items\roi.py", line 371, in _createLabelItem
        markerPos = self._getLabelPosition()
File "C:\Users\Pierre\AppData\Roaming\Python\Python36\site-packages\silx\gui\plot\items\roi.py", line 892, in _getLabelPosition
        return points.min(axis=0)
AttributeError: 'NoneType' object has no attribute 'min'
--


PiRK avatar Jul 11 '19 10:07 PiRK

Capture

PiRK avatar Jul 11 '19 10:07 PiRK

Providing a geometry in __init__ (or a using a default one) sounds reasonable.. but would probably require quite some refactoring (not sure).

Here is a fix that makes it works:

--- a/silx/gui/plot/items/roi.py
+++ b/silx/gui/plot/items/roi.py
@@ -315,6 +315,8 @@ class RegionOfInterest(qt.QObject):
         itemIndex = 0
 
         controlPoints = self._getControlPoints()
+        if controlPoints is None:
+            return
 
         if self._labelItem is None:
             self._labelItem = self._createLabelItem()

I only quickly tested it. If you can test it and it's fine, we can make a PR out of it.

t20100 avatar Jul 12 '19 09:07 t20100