[FIX] ZeroDivisionError owmosaic.py #2046
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2046 +/- ##
==========================================
- Coverage 70.69% 70.66% -0.04%
==========================================
Files 315 315
Lines 53900 53834 -66
==========================================
- Hits 38106 38040 -66
Misses 15794 15794Continue to review full report at Codecov.
|
|
|
||
| def test_zerodivision_error(self): | ||
| """ | ||
| ZeroDivisionError should not be thrown when |
There was a problem hiding this comment.
Could you make this description more explicit. Instead of "some kind of invalid data" try to describe what the problem in data was.
| try: | ||
| self.send_signal("Data", table) | ||
| except ZeroDivisionError: | ||
| self.assertTrue(False) |
There was a problem hiding this comment.
self.fail can be used to make the test fail. And it is also a good idea to specify the reason of the failure ("Widget crashed with ZeroDivisionError" or something like this)
Orange/widgets/visualize/owmosaic.py
Outdated
| @@ -512,6 +512,18 @@ def update_graph(self): | |||
| spacing = self.SPACING | |||
| bar_width = self.BAR_WIDTH | |||
|
|
|||
| def get_counts(attr_vals, values): | |||
| if attr_vals == "": | |||
There was a problem hiding this comment.
Nice. Could you also add a docstring to this function describing what it does and how it handles zero counts?
Orange/widgets/visualize/owmosaic.py
Outdated
| @@ -512,6 +512,18 @@ def update_graph(self): | |||
| spacing = self.SPACING | |||
| bar_width = self.BAR_WIDTH | |||
|
|
|||
| def get_counts(attr_vals, values): | |||
| if attr_vals == "": | |||
| counts = [conditionaldict[val] for val in values] | |||
There was a problem hiding this comment.
While draw_data used conditionaldict[val], draw_text used conditionaldict.get(val, 1). Could you elaborate on why indexing is sufficient?
| try: | ||
| self.send_signal("Data", table) | ||
| except ZeroDivisionError: | ||
| self.fail("Widget crashed due to ZeroDivisionError.") |
There was a problem hiding this comment.
Using data with only NaNs should not result in any kind of error, not just ZeroDivision.
Why not make this test more general: rename from test_zerodivision_error to e.g. test_nan_column, remove the try, all exceptions will be shown when/if they happen.
ZeroDivisionError should not be thrown when a column with only NaN-s is loaded into widget. https://sentry.io/biolab/orange3/issues/204400244/ Corrects the total value and rectangles' widths.
Issue
ZeroDivisionError is thrown when invalid data is loaded into widget. For instance: [[0, NaN], [NaN, NaN]].
https://sentry.io/biolab/orange3/issues/204400244/
Description of changes
If the value "total" is zero aborts drawing the plot.
Includes