[FIX] owfile: Prevent setting empty feature names #2039
Conversation
|
Could you give me the steps to reproduce this issue? |
|
Put File and Scatter Plot widget and then connect them. Then open some data and delete a column name. Click Apply button and close the window and then the application. You do not need to save your work. The Pickle Error should pop up. |
|
@BlazZupan suggested a better way to solve this problem. That is preventing a user to even write a blank column name (string with zero length or only with white spaces). I am now doing some changes in file domaineditor.py. |
|
As it is, this warning should be an error and not allow the user to proceed. EDIT: I was a couple of minutes too late and didn't see the previous reply - I agree with @BlazZupan's comment (+ the above still holds) |
Codecov Report
@@ Coverage Diff @@
## master #2039 +/- ##
==========================================
+ Coverage 70.7% 70.72% +0.01%
==========================================
Files 343 343
Lines 54478 54486 +8
==========================================
+ Hits 38519 38533 +14
+ Misses 15959 15953 -6Continue to review full report at Codecov.
|
|
@lanzagar: Orange already auto-set the empty names to Feature 1, Feature 2, etc. if there are such columns in a file. |
| @@ -127,3 +127,15 @@ def test_file_not_found(self): | |||
| # Open a sample dataset | |||
| self.open_dataset("iris") | |||
| self.assertFalse(self.widget.Error.file_not_found.is_shown()) | |||
|
|
|||
| def test_check_column_noname(self): | |||
| ''' | |||
Orange/widgets/utils/domaineditor.py
Outdated
| @@ -80,7 +80,8 @@ def setData(self, index, value, role): | |||
| row_data = self.variables[row] | |||
| if role == Qt.EditRole: | |||
| if col == Column.name: | |||
| row_data[col] = value | |||
| if not (value.isspace() or value == ""): | |||
There was a problem hiding this comment.
This results in the above if col == Column.name succeeding and then the inside if doing nothing for empty values. But the function still returns True in the end.
Instead, using if col == Column.name and not (value.isspace() or value == "") would result either in the value being updated or nothing happens and function returns False. Which means that after an "incorrect" edit the Apply button does not become enabled (same as cancelling the edit with Esc).
| chain([(at, Place.feature) for at in domain.attributes], | ||
| [(cl, Place.class_var) for cl in domain.class_vars], | ||
| [(mt, Place.meta) for mt in domain.metas])): | ||
| chain([(at, Place.feature) for at in domain.attributes], |
There was a problem hiding this comment.
Why the additional indent here?
There was a problem hiding this comment.
PYLINT demands some additional spaces.
PickleError thrown if a column does not have a name. Column name cannot be changed to an empty string or a string with whitespaces.
lanzagar
changed the title
Issue
PickleError thrown if a column does not have a name.
Description of changes
Column name cannot be changed to an empty string or a string with whitespaces.
Includes