[ENH] Automated and better summaries#5308
Conversation
| return '<hr>'.join(full_details) | ||
|
|
||
|
|
||
| @summarize.register(Table) |
There was a problem hiding this comment.
If we stop supporting Python 3.6, it would suffice to annotate the function's argument as Table instead of passing Table to decorator.
There was a problem hiding this comment.
According to NEP 29, we've been free to drop 3.6 since Jun 23, 2020. The only concern would be Anaconda (for the same reason we still tacitly support PyQt5.9).
https://numpy.org/neps/nep-0029-deprecation_policy.html
087a2ae to
e2d90dc
Compare
3fbbfe9 to
ef9fc8f
Compare
7365d18 to
514f857
Compare
|
/rebase |
|
Rebase alone won't help. This requires a new release of orange-widget-base because of |
| self._invalidated = True | ||
| self._domain_invalidated = True | ||
| self.input_changed.connect(self.set_input_summary) | ||
| self.output_changed.connect(self.set_output_summary) |
There was a problem hiding this comment.
Are the signals (input_changed and output_changed) still needed?
There was a problem hiding this comment.
You tell me.
I'd prefer if they're eliminated. (Otherwise every widget could have them.)
There was a problem hiding this comment.
Please keep them. I know we separated the repositories, but we need the API between canvas/widget-base. I'd love to be able to read/write a widget's title from OWWidget code, too.
There was a problem hiding this comment.
@irgolic, this signal doesn't help you a lot. This is OWDataProjectionWidget, not OWBaseWidget. You want to keep it here or have it there?
There was a problem hiding this comment.
Sorry, I got it confused with this. https://orange-canvas-core.readthedocs.io/en/0.1.16/orangecanvas/scheme.events.html#orangecanvas.scheme.events.WorkflowEvent.OutputLinkAdded
By all means, remove these signals. :)
|
widget-base is now released. |
23e80da to
7954dd5
Compare
Codecov Report
@@ Coverage Diff @@
## master #5308 +/- ##
==========================================
- Coverage 86.37% 86.15% -0.22%
==========================================
Files 303 303
Lines 62157 61592 -565
==========================================
- Hits 53688 53065 -623
- Misses 8469 8527 +58 |
|
@VesnaT, I think this is ready (rebased, corrected and linted). |
VesnaT
left a comment
There was a problem hiding this comment.
Some observations:
CSV File Importwidget does not display the summary- the summary code can be removed from
Aggregate columnswidget
|
|
||
| summarize_by_name(Model, "⛄" if date.month == 12 else "🄼") | ||
| summarize_by_name(Learner, "🄻") | ||
| summarize_by_name(Preprocess, "🄿") |
There was a problem hiding this comment.
Have you considered displaying all preprocessors in case of PreprocessorList?
There was a problem hiding this comment.
I haven't. Now I did. Names are not nice -- they are class names. But that's all we have.
| def _get_details(self): | ||
| details = "Data:<br>" | ||
| details += format_summary_details(self.data).replace('\n', '<br>') if \ | ||
| details += format_summary_details(self.data, format=Qt.RichText) if \ |
There was a problem hiding this comment.
The if part can now be removed.
| self.__pending_selection = self.selection | ||
| self._invalidated = True | ||
| self._domain_invalidated = True | ||
| self.input_changed.connect(self.set_input_summary) |
There was a problem hiding this comment.
The signals are no longer user and can be removed.
And I thought I checked them all. The problem was that it used old-style output signals. I migrated it to new signals.
This widget was moved from prototypes after I prepared this PR. :) Fixed. |
Co-authored-by: Rafael Irgolic <hello@irgolic.com>
Issue
Ref: #5108
Requires: biolab/orange-widget-base#136.
Description of changes
See biolab/orange-widget-base#136.
Includes