[ENH] ROC analysis: show thresholds

[matejklemen]

Issue

Implements #3057.

Description of changes

When mouse is put over a point on ROC curve, the threshold that was used for calculation now appears. If threshold value is nan, the value does not get shown. Also, the displaying of thresholds is currently not implemented when showing individual curves.

My questions:

• currently thresholds are displayed when mouse goes 'perfectly' over a point - should this be changed so that they get displayed if mouse is in some very small radius of the point?
• what kind of a test do I write for this functionality (i. e. mouse events) - if any?

General comments are also welcome.

Includes

• Code changes
• Tests
• Documentation

[ales-erjavec]

If I enable "Show convex ROC curves" and hover over the graph I get:

-------------------------- AttributeError Exception ---------------------------
Traceback (most recent call last):
  File "/Users/aleserjavec/workspace/orange3/Orange/widgets/evaluate/owrocanalysis.py", line 614, in _on_mouse_moved
    pts = sp.pointsAt(act_pos)
AttributeError: 'PlotCurveItem' object has no attribute 'pointsAt'
-------------------------------------------------------------------------------

[ales-erjavec]

Connect the signal once in the __init__ method:
self.plotview.scene().sigMouseMoved.connect(self._on_mouse_moved)

[ales-erjavec]

Why not QToolTip.showText?

[matejklemen]

Mainly because I am new to QT and was just following a few resources online (I've changed it now).
I've fixed most of the things that you've mentioned and some other bugs.
Thank you for the helpful comments!

[codecov-io]

Codecov Report

Merging #3172 into master will increase coverage by 0.01%.
The diff coverage is 90.47%.

@@            Coverage Diff             @@
##           master    #3172      +/-   ##
==========================================
+ Coverage   82.74%   82.76%   +0.01%     
==========================================
  Files         344      344              
  Lines       59304    59397      +93     
==========================================
+ Hits        49073    49158      +85     
- Misses      10231    10239       +8

[ales-erjavec]

This assumes implicitly that the order of the curves in the self.plot.curves corresponds to the self.selected_classifiers. This seems fragile.

You can use:

curves = [(clsf_idx, self.plot_curves(self.target_index, clsf_idx), self.curve_data(self.target_index, clsf_idx))
          for clsf_idx in self.selected_classifiers]  # type: List[Tuple[int, plot_curves, ROCData]]

to retrieve triples of (classifier_index, plot_curves, curve_data). Use plot_curves similarly as curve_data in ROC averaging dispatch below, but use

plot_curves.merge().curve_item
plot_curves.avg_vertical().curve_item
plot_curves.avg_threshold().curve_item

to get the pg.PlotCurveItem instances

[matejklemen]

My bad - should be fixed now. I've also fixed some other bugs:

• if mouse was hovered over a point on curve for some classifier and then the same classifier's curve would be hidden and mouse hovered over the same point as before, the cache would not reset and therefore an invalid threshold would be shown,
• if tooltip was not being shown and the cache point was "valid" (i. e. same point as previously shown), a tooltip with message "" would be created, which resulted in hiding a tooltip.

I have also encountered the following exception (just once), which I can not seem to be able to reproduce anymore:

--------------------------- RuntimeError Exception ----------------------------
Traceback (most recent call last):
  File "/home/matej/.local/lib/python3.5/site-packages/pyqtgraph/GraphicsScene/GraphicsScene.py", line 162, in mouseMoveEvent
    self.sendHoverEvents(ev)
  File "/home/matej/.local/lib/python3.5/site-packages/pyqtgraph/GraphicsScene/GraphicsScene.py", line 228, in sendHoverEvents
    items = self.itemsNearEvent(event, hoverable=True)
  File "/home/matej/.local/lib/python3.5/site-packages/pyqtgraph/GraphicsScene/GraphicsScene.py", line 424, in itemsNearEvent
    if hoverable and not hasattr(item, 'hoverEvent'):
RuntimeError: wrapped C/C++ object of type ScatterPlotItem has been deleted

When browsing through old issues, I've found out that there's been some instances of similar issues (#1316), though I can't say I completely understand your fix for this.

[ales-erjavec]

This loop could be vectorized

roc_points = numpy.column_stack((curve_pts.fpr, curve_pts.tpr))
query_pt = numpy.array([[mouse_pt.x(), mouse_pt.y()]])
diff = roc_points - query_pt
idx_closest = numpy.argmin(numpy.linalg.norm(diff, axis=1))

(... although a slightly moot point as the sp.pointsAt call above already runs through all the points in a python loop)

[matejklemen]

True - fixed.
I've also made another minor change: now QCursor.pos() is directly used everywhere instead of using it directly in some cases and in the other cases saving it and then using the variable (just a consistency fix).

[ales-erjavec]

currently thresholds are displayed when mouse goes 'perfectly' over a point - should this be changed so that they get displayed if mouse is in some very small radius of the point?

I think the hit radius is sufficient as it is. However it might be better to collect points from all the curves under the mouse and display a list of all thresholds with classifier names. Right now if two or more curves are close to each other it is not clear to which does the shown threshold belong to.

what kind of a test do I write for this functionality (i. e. mouse events) - if any?

I would use QTest.mouseMove targeting the QGraphicsView's viewport (plotview.viewport() in the OWROCAnalysis). Try to map one of the curve's points to scene coordinates (QGraphicsItem.mapToScene) and those to the viewport (QGraphicsWidget.mapFromScene).

[matejklemen]

I've improved the tooltips so that it is clear, which classifier a threshold belongs to. Also, the tooltip now shows thresholds for all overlapping points.

I'll see what I can do about the tests.

Edit: I'm having a bit of trouble creating the tests - so far this is what I've got:

# setup
data_in = Orange.data.Table("titanic")
res = Orange.evaluation.TestOnTrainingData(
    data=data_in,
    learners=[Orange.classification.KNNLearner(),
              Orange.classification.LogisticRegressionLearner()],
    store_data=True
)

self.send_signal(self.widget.Inputs.evaluation_results, res)
self.widget.roc_averaging = OWROCAnalysis.Merge
self.widget.target_index = 0
self.widget.selected_classifiers = [0, 1]
self.widget._replot()

# curve data for classifier 0
curve = self.widget.plot_curves(self.widget.target_index, 0)
curve_merge = curve.merge()

QTest.mouseMove(self.widget.plotview.viewport(), curve_merge.curve_item.mapToScene(0, 0))
# example assertion
self.assertTrue(QToolTip.isVisible())

The QTest.mouseMove(...) seems to work ok when manually testing - it throws the cursor to (0, 0) (in plot coordinates), but does not trigger the mouse callback in the test.

[ales-erjavec]

The QTest.mouseMove(...) seems to work ok when manually testing - it throws the cursor to (0, 0) (in plot coordinates), but does not trigger the mouse callback in the test.

Maybe QTBUG-5232. I swear I saw this when writing https://github.com/biolab/orange3/pull/3014/files#diff-da80849ceff100f955c9ec05920ba089R138.

If I use that function and modify your test like this:

        ...
        # curve data for classifier 0
        curve = self.widget.plot_curves(self.widget.target_index, 0)
        curve_merge = curve.merge()

        view = self.widget.plotview
        item = curve_merge.curve_item  # type: pg.PlotCurveItem
        xs, ys = item.getData()
        x, y = xs[len(xs) // 2], ys[len(ys) // 2]
        pos = item.mapToScene(x, y)
        pos = view.mapFromScene(pos)
        mouseMove(view.viewport(), pos)
        # example assertion
        self.assertTrue(QToolTip.isVisible())

then the test passes. Can you rebase on current master and use the mouseMove function from test_combobox.py (move it to e.g. Orange/widgets/tests/utils.py and import it from there)

[matejklemen]

Thank you, that does fix the issue.

I've stumbled upon another issue though - the points which I'm trying to move the mouse to are not being mapped correctly. For example, if I try to simulate a mouse move to (x=0.0, y=0.0) and (x=1.0, y=1.0), it produces the same threshold/tooltip.

I've noticed that these two points get mapped to very similar positions inside the test (QPoint(56, 1) and QPoint(57, 2) in my case), while during "manual" testing they get mapped far apart and correctly.
My guess is that this is happening because the widget, created inside the test, is tiny, and therefore everything is getting mapped to pretty much the same coordinates - but I'm not sure how to fix it.

[ales-erjavec]

Try adding

vb = self.widget.plot.getViewBox()
vb.childTransform()  # Force pyqtgraph to update transforms

somewhere before mapping the points.

[matejklemen]

Thanks once again. 😃
I've moved the mouseMove(...) function and added tests for tooltips - hopefully I didn't mess anything up.

[ales-erjavec]

I do not understand why the tooltip cache is necessary. Can you add a comment explaining the reasoning for its use?

[matejklemen]

My reasoning behind it was that instead of checking all points of a curve to find the closest one, it would first check the tooltip cache, which contains at most 1 threshold (point) per (selected) classifier.

It's hard to say if it's worth the additional 20-ish lines of code though.

[ales-erjavec]

But it already checks all points in pts = sp.pointsAt(act_pos)

[matejklemen]

Right, but it also does it again later (although it is vectorized): idx_closest = numpy.argmin(numpy.linalg.norm(diff, axis=1)).

Considering your point, it does seem that the performance gain from tooltip cache might not be as big as I thought. Should I remove it and make the code clearer that way?

Edit: also, the tooltip cache provides a convenient way to test the contents of tooltips - the first alternative way I can think of would be to test the entire tooltip text, e.g. assertEqual("Tooltips:\n(#1) 0.400", QTooltip.text()).

[ales-erjavec]

Should I remove it and make the code clearer that way?

It does not matter really, you can leave it.