[FIX] Select Rows: None on output when no data#2726
[FIX] Select Rows: None on output when no data#2726janezd merged 2 commits intobiolab:masterfrom jerneju:rank-value-max
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2726 +/- ##
==========================================
+ Coverage 76.03% 76.04% +0.01%
==========================================
Files 338 338
Lines 59628 59641 +13
==========================================
+ Hits 45339 45356 +17
+ Misses 14289 14285 -4 |
janezd
left a comment
There was a problem hiding this comment.
A comment for other potential reviewers: it is not clear what widget should output in this case. @jerneju proposed None and I agreed. It makes sense from the user perspective: with None, the canvas shows that the widgets hasn't output anything. Otherwise, the canvas indicates that the widget has output something ... but this something is empty, which is a confusing non-difference for the user.
None is also better technically since most widgets do not expect empty tables and, worse, empty domains on inputs. (This is not to say that a well-behaved widget should not crash if this happens.)
Orange/widgets/data/owselectrows.py
Outdated
| self.Outputs.matching_data.send(matching_output) | ||
| self.Outputs.unmatched_data.send(non_matching_output) | ||
| def output_filter(output): | ||
| return output if output is not None and len(output) else None |
There was a problem hiding this comment.
I here agree with @astaric's usual comment about my code. If-else expressions in Python are mostly unreadable. I'd prefer
if output is not None and len(output):
return output
You don't even need return None here, but you may add it to make it explicit.
There was a problem hiding this comment.
Oh, besides, I don't like the function at all. I'd prefer having
if not len(matching_output):
matching_output = None
and the same for non_matching_output.
The same amount of code, but way more explicit.
|
I made the change. Another option is to allow empty tables, but not empty domains. In this case we don't call |
Issue
Rank widget crashes because receives empty
Tableand emptyDomainwhich is created by Select Rows.Description of changes
Select Rows commits
None.Includes