[FIX] File: Raise and handle Exc. when file bad pickle #2232
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2232 +/- ##
==========================================
- Coverage 72.26% 72.26% -0.01%
==========================================
Files 318 318
Lines 54856 54864 +8
==========================================
+ Hits 39644 39649 +5
- Misses 15212 15215 +3Continue to review full report at Codecov.
|
Orange/data/io.py
Outdated
| return pickle.load(f) | ||
| table = pickle.load(f) | ||
| if not isinstance(table, Table): | ||
| raise Exception("Not a table.") |
There was a problem hiding this comment.
Don't raise instances of Exception; Exception is just the base class for (most) other exceptions. I guess that TypeError would be the appropriate type of exception for this.
You can't even catch Exception without pylint complaining (unless silenced).
There was a problem hiding this comment.
Change the message to "file does not contain a data table". It's more informative and, like most Python exceptions, starts with a lower case and has no period.
| @@ -195,3 +195,21 @@ def test_check_datetime_disabled(self): | |||
| for i in range(4): | |||
| vartype_delegate.setEditorData(combo, idx(i)) | |||
| self.assertEqual(combo.count(), counts[i]) | |||
|
|
|||
| def test_open_bad_pickle(self): | |||
There was a problem hiding this comment.
This is not the proper test for this code (and not the proper place for the test function itself). You modified Orange.data.io.PickleReader, so you should test Orange.data.io.PickleReader and not a widget. You should always aim to test as little code as possible. The test should not rely on the way in which a widget that uses the reader handles exceptions.
Second, simplify this test. You don't need temporary files. Instead of writing a temporary pickle file, just patch open and pickle.load to return None. Then call self.assertRaises(TypeError, PickleReader.read, "foo"). You'll do everything in three simple lines.
Raise TypeError when PickleReader reads a pickle file without a table (and it suppose to be there). https://sentry.io/biolab/orange3/issues/208584693/
Issue
https://sentry.io/biolab/orange3/issues/208584693/
Description of changes
Raise TypeError when PickleReader reads a pickle file without a table (and it suppose to be there).
Includes