[FIX] File: Provide percent missing values in Info box #3305
Conversation
MaxGreil
changed the title
|
@markotoplak can you please have a look at this? |
Orange/widgets/data/owfile.py
Outdated
| miss = "%.1f" % table.get_nan_percentile_class() | ||
| text += " ({}% missing values)".format(miss) | ||
| else: | ||
| text += " (no missing values)" |
There was a problem hiding this comment.
These five lines repeat twice. Could you construct this part of the string beforeif (but only if data has class of any kind)?
There was a problem hiding this comment.
I don't understand what you have in mind here. Which if do you mean and how should I then construct the string before it?
Orange/widgets/data/owfile.py
Outdated
| format(len(domain.class_var.values)) | ||
| if table.has_missing_class(): | ||
| miss = "%.1f" % table.get_nan_percentile_class() | ||
| text += " ({}% missing values)".format(miss) |
There was a problem hiding this comment.
Why not something like
text += " ({:.1f}% missing values)".format(table.get_nan_percentile_class())
Or
miss = table.get_nan_percentile_class()
text += " ({:.1f}% missing values)".format(miss)
, so that we don't format the string in two lines in two different ways.
Orange/widgets/data/owfile.py
Outdated
| miss = "%.1f" % table.get_nan_percentile_class() | ||
| text += " ({}% missing values)".format(miss) | ||
| else: | ||
| text += " (no missing values)" | ||
| elif table.domain.class_vars: | ||
| text += "<br/>Multi-target; {} target variables.".format( | ||
| len(table.domain.class_vars)) |
There was a problem hiding this comment.
If you extract the above five lines, you could also report the percentage of unknowns for this case (there's no reason why this case should be different).
Orange/data/table.py
Outdated
| return np.isnan(self.X).sum() / self.X.size * 100 | ||
|
|
||
| def get_nan_percentile_class(self): | ||
| return np.isnan(self._Y).sum() / self._Y.size * 100 |
There was a problem hiding this comment.
I think it would be better if these two functions reported relative frequencies, not percentages. That is, I would remove * 100. Percentages are more "human-friendly", while these two functions look general and could also be used in other, non-GUI-related code.
Codecov Report
@@ Coverage Diff @@
## master #3305 +/- ##
==========================================
- Coverage 82.57% 82.35% -0.22%
==========================================
Files 348 360 +12
Lines 61395 64197 +2802
==========================================
+ Hits 50697 52871 +2174
- Misses 10698 11326 +628 |
| def has_missing_class(self): | ||
| """Return `True` if there are any missing class values.""" | ||
| return bn.anynan(self._Y) | ||
|
|
||
| def get_nan_frequency_attribute(self): | ||
| return np.isnan(self.X).sum() / self.X.size |
There was a problem hiding this comment.
Sorry, I just noticed one more: could you return 0 if self.X.size is 0, to avoid division by zero? Same in the next function?
|
Thank you both. I also added some tests. |
Issue
Fixes issue #3157.
Description of changes
Info Box in widget File now displays the same information as the info box in widget Data Table
Includes