[FIX] Removed header types and flags from .csv and .tab #3427
Conversation
AndrejaKovacic
changed the title
AndrejaKovacic
changed the title
AndrejaKovacic
changed the title
AndrejaKovacic
force-pushed
the
fix_save
branch
from
a53a4d9
to
fc34931
Compare
Codecov Report
@@ Coverage Diff @@
## master #3427 +/- ##
==========================================
+ Coverage 83.58% 83.71% +0.12%
==========================================
Files 368 370 +2
Lines 65812 66304 +492
==========================================
+ Hits 55010 55504 +494
+ Misses 10802 10800 -2 |
There was a problem hiding this comment.
I suppose FileWrite should be able to indicate whether it supports optional type annotations. We have to think about it, but I guess we could have a flag (class attribute) OPTIONAL_TYPE_ANNOTATIONS = False. If False, it may mean that annotations are always (like in pickle) or never present. If True, the widget enables the checkbox and passes the additional argument to write.
Orange/data/io.py
Outdated
| @@ -798,11 +798,12 @@ def header_flags(data): | |||
| zip(repeat('meta'), data.domain.metas))))) | |||
|
|
|||
| @classmethod | |||
| def write_headers(cls, write, data): | |||
| def write_headers(cls, write, data, with_annotations): | |||
There was a problem hiding this comment.
Maybe with_annotations=True?
Orange/data/io.py
Outdated
| @@ -1062,7 +1060,7 @@ def write_graph(cls, filename, graph): | |||
| tree.export_graphviz(graph, out_file=cls.open(filename, 'wt')) | |||
|
|
|||
| @classmethod | |||
| def write(cls, filename, tree): | |||
| def write(cls, filename, tree, with_annotations): | |||
There was a problem hiding this comment.
Maybe with_annotations=True.
Orange/widgets/data/owsave.py
Outdated
| @@ -89,6 +90,10 @@ def __init__(self): | |||
|
|
|||
| box.layout().addLayout(form) | |||
|
|
|||
| self.annotations_cb = gui.checkBox( | |||
| self.controlArea, self, "with_annotations", label="Save with Orange annotations", | |||
There was a problem hiding this comment.
Maybe rename the setting to Add type annotations.
Orange/widgets/data/owsave.py
Outdated
| @@ -175,14 +180,18 @@ def save_file(self): | |||
| os.path.join( | |||
| self.last_dir, | |||
| self.basename + self.type_ext + self.compress_ext), | |||
| self.data) | |||
| self.data, self.with_annotations, | |||
There was a problem hiding this comment.
This breaks compatibility with existing additional writers (if there are any!) that don't accept this argument. Writers that do not support it, should be called without it (see the general comment).
Orange/widgets/data/owsave.py
Outdated
| except Exception as err_value: | ||
| self.error(str(err_value)) | ||
| else: | ||
| self.error() | ||
|
|
||
| def update_extension(self): | ||
| self.type_ext = [ext for name, ext, _ in FILE_TYPES if name == self.filetype][0] | ||
| self.annotations_cb.setEnabled(True) | ||
| if self.type_ext in ['.pkl', '.xlsx']: |
There was a problem hiding this comment.
Whether this checkbox is shown should be decided by the file writer and not hard coded in the widget. See the general comment.
AndrejaKovacic
changed the title
| self.annotations_cb = gui.checkBox( | ||
| self.controlArea, self, "add_type_annotations", label="Add type annotations", | ||
| ) | ||
|
|
There was a problem hiding this comment.
You can fix the layout (checkbox not in the box, but outside and with a smaller indent) by changing this to:
self.annotations_cb = gui.checkBox(
None, self, "add_type_annotations", label="Add type annotations",
)
form.addRow(self.annotations_cb, None)
The layout of this widget is awful anyway, but this is another issue.
Orange/data/io.py
Outdated
| cls.write_data(writer.writerow, data) | ||
| cls.write_table_metadata(filename, data) | ||
| if with_annotations: |
There was a problem hiding this comment.
I would remove this if - saving the additional metadata file is not related to type annotations for variables.
|
Two post-merge comments. Use one of these keywords in PR's description, not "Addresses", and the related issue will be closed automatically. Write commit messages in imperative mood. See, e.g. this or this. We don't have a habit of going past the short message, but formulate them as prescribed on the above pages. |
Issue
Addreses #2717.
Description of changes
When saving, header types and flags are no loger saved in .csv and .tab.
Includes