-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] data/io.py Metadata file not saved anymore when it is empty #2002
Conversation
Codecov Report@@ Coverage Diff @@
## master #2002 +/- ##
=======================================
Coverage 89.53% 89.53%
=======================================
Files 91 91
Lines 9199 9199
=======================================
Hits 8236 8236
Misses 963 963 Continue to review full report at Codecov.
|
Orange/tests/test_metadata_save.py
Outdated
table = Table("iris") | ||
fname = path.join(tempdir, "out.tab") | ||
TabReader.write_table_metadata(fname, table) | ||
self.assertEqual(path.isfile(fname + ".metadata"), True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use self.assertTrue
Orange/tests/test_metadata_save.py
Outdated
table = Table("titanic") | ||
fname = path.join(tempdir, "out.tab") | ||
TabReader.write_table_metadata(fname, table) | ||
self.assertEqual(path.isfile(fname + ".metadata"), False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use self.assertFalse
Orange/data/io.py
Outdated
@@ -373,7 +373,7 @@ def write(cls, filename, data): | |||
|
|||
@classmethod | |||
def write_table_metadata(cls, filename, data): | |||
if isinstance(filename, str) and hasattr(data, 'attributes'): | |||
if isinstance(filename, str) and hasattr(data, 'attributes') and data.attributes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasattr(data, 'attributes') and data.attributes
could be simplified to
getattr(data, 'attributes', None)
Orange/tests/test_metadata_save.py
Outdated
@@ -0,0 +1,36 @@ | |||
# Test methods with long descriptive names can omit docstrings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a special reason this tests were not placed in Orange/tests/test_tab_reader.py
? They only test TabReaders functionality and all other TabReader tests are in that file.
Orange/tests/test_metadata_save.py
Outdated
DiscreteVariable._clear_cache() | ||
|
||
def test_metadata(self): | ||
tempdir = tempfile.mkdtemp() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests assume that iris has metadata and titanic has not. If this would change in the future (if we added metadata for titanic as well), the second test would fail and will need to be fixed, even though the change would have nothing to do with the saving of metadata.
I suggest that you manually set the attributes dict to whatever you need in a test and then do the testing. This way, attributes dict will be the one you expect it to be, even if the datasets change in the future.
Issue Do not save the metadata file (when saving the data) if this is empty (0 Bytes) Description of changes Change if statement and add third condition. It implies that attribute should not be empty.
Issue
Do not save the metadata file (when saving the data) if this is empty (0 Bytes)
Description of changes
Change if statement and add third condition. It implies that attribute should not be empty. Add test file to check if metadata is saved when empty
Includes