-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] Reporting tabular in Data Table and Rank widgets #1573
Conversation
Current coverage is 88.66% (diff: 100%)@@ master #1573 diff @@
==========================================
Files 78 78
Lines 8124 8124
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 7203 7203
Misses 921 921
Partials 0 0
|
Ok. But I think you should still handle the case where the data(Qt.BackgroundRole) is not a QBrush in |
90ebbf0
to
1980720
Compare
In Qt, QBrush is always returned for this role. No guarantee, but a convention. If the user passes arbitrary types for predefined roles, that's a bug waiting to be discovered.
Emphasis added.
|
1980720
to
375e591
Compare
Thanks for the elaborate response. Please find the PR completely revised. |
830790c
to
19f4071
Compare
@@ -216,16 +219,18 @@ def data(role=Qt.DisplayRole, | |||
if row is None or col is None: | |||
return model.headerData(col if row is None else row, | |||
orientation, role) | |||
return model.data(model.index(row, col), role) | |||
return try_fmtnum(model.data(model.index(row, col), role)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not reformat the number here, at least not like this.
Orange used to keep track of the number of decimal places it should print for a specific variable. For instance, the "ST by exercise" has only a single decimal in the heart_disease.tab file, and the "age" and the "number of vessels colored" have none, obviously. Orange used to record this when loading the data. Now all numeric values have three decimals.
I consider this a bug (#1581). After this is fixed, model.data
that is used in, say, the Table widget, will (again) show the numeric values with appropriate number of decimals, so this function should not reformat it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased and without the offending commit.
They should not be overriden, unless there is some non-obvious reason, in order to make report unittest work for this widget.
19f4071
to
20c6e1e
Compare
QBrush. https://doc.qt.io/qt-4.8/qt.html#ItemDataRole-enum
Fixes report in Data Table.