[FIX] Test and Score: Sort numerically, not alphabetically

[janezd]

Fixes #3947 by using a QSortFilterProxy that sorts numerically, if column values can be converted to numbers, or case-insensitive, if they can't.

Includes

• Code changes
• Tests

[codecov]

Codecov Report

Merging #3951 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3951      +/-   ##
==========================================
- Coverage   85.22%   85.13%   -0.09%     
==========================================
  Files         382      378       -4     
  Lines       67595    67159     -436     
==========================================
- Hits        57606    57176     -430     
+ Misses       9989     9983       -6

[ales-erjavec]

Or maybe just undo b419679 and go on from there.

diff --git a/Orange/widgets/evaluate/owtestlearners.py b/Orange/widgets/evaluate/owtestlearners.py
index 139f2cb9c..9a2d77de9 100644
--- a/Orange/widgets/evaluate/owtestlearners.py
+++ b/Orange/widgets/evaluate/owtestlearners.py
@@ -533,7 +533,7 @@ class OWTestLearners(OWWidget):
                 for stat, scorer in zip(stats, self.scorers):
                     item = QStandardItem()
                     if stat.success:
-                        item.setText("{:.3f}".format(stat.value[0]))
+                        item.setData(float(stat.value[0]), Qt.DisplayRole)
                     else:
                         item.setToolTip(str(stat.exception))
                         if scorer.name in self.score_table.shown_scores:
diff --git a/Orange/widgets/evaluate/utils.py b/Orange/widgets/evaluate/utils.py
index ebe060327..6caa6811b 100644
--- a/Orange/widgets/evaluate/utils.py
+++ b/Orange/widgets/evaluate/utils.py
@@ -109,6 +109,12 @@ class ScoreTable(OWComponent, QObject):
             size = super().sizeHint(*args)
             return QSize(size.width(), size.height() + 6)
 
+        def displayText(self, value, locale):
+            if isinstance(value, float):
+                return "{:.3f}".format(value)
+            else:
+                return super().displayText(value, locale)
+
     def __init__(self, master):
         QObject.__init__(self)
         OWComponent.__init__(self, master)

[janezd]

Thanks @ales-erjavec. I reverted the changes in b419679, as you proposed.

I still kept a (now simpler) proxy because I like putting the failed tests at the bottom.

[thocevar]

Sorting has a problem with np.nan because it is a float.

A stable sort would be nice for sorting by multiple criteria (e.g. if the first criteria reaches the max value). Sorting by name and then by some constant column does not retain the alphabetical order. I don't know why this doesn't work out of the box.

This would also solve the problem with a changing order of failed tests. If you have several, they change positions when going from ascending to descending (but stay in place from descending to ascending).

[janezd]

Sorting has a problem with np.nan because it is a float.

There should be no nans in this table. If there are (but I haven't seen them yet), this is a different bug; they have to be replaced by errors by the widget.

A stable sort would be nice for sorting by multiple criteria (e.g. if the first criteria reaches the max value).

Sorting is done by QSortFilterProxyModel. My class provides just the comparison, I can't control the algorithm.

This would also solve the problem with a changing order of failed tests.

It would be nice to keep the order, but it's not such a problem. :)

[thocevar]

There should be no nans in this table. If there are (but I haven't seen them yet), this is a different bug; they have to be replaced by errors by the widget.

True, but as it is a part of the utils, it would be nice if it handled this additional case (besides "" and None) that might come up somewhere else. Actually, anything that is not a float could fall into the same bottom category.

Sorting is done by QSortFilterProxyModel. My class provides just the comparison, I can't control the algorithm.

I've just checked that QSortFilterProxyModel behaves as a stable sort in the Predictions widget. Now I can't find the case, where it doesn't, so maybe it does. :)

[janezd]

There should be no nans in this table.

True, but as it is a part of the utils, it would be nice if it handled this additional case (besides "" and None) that might come up somewhere else. Actually, anything that is not a float could fall into the same bottom category.

ScoreModel is supposed to contain scores, and these cannot (should not) be nan. But you're right: it's better to support them to avoid problems in the future and particularly to avoid bugs if somebody copies this code or moves it to some more general utils module. Done. The changed code is also cleaner.