From fe16408ef238a913ce850d205e805746c2167d54 Mon Sep 17 00:00:00 2001 From: janezd Date: Fri, 18 Jan 2019 23:39:50 +0100 Subject: [PATCH 01/12] OWSave: Improve GUI --- Orange/data/io.py | 2 +- Orange/widgets/data/owsave.py | 253 +++++++++++++++------------------- 2 files changed, 112 insertions(+), 143 deletions(-) diff --git a/Orange/data/io.py b/Orange/data/io.py index 8236b13722e..3efab04b5fb 100644 --- a/Orange/data/io.py +++ b/Orange/data/io.py @@ -987,7 +987,7 @@ class ExcelReader(FileFormat): """Reader for excel files""" EXTENSIONS = ('.xlsx',) DESCRIPTION = 'Microsoft Excel spreadsheet' - SUPPORT_COMPRESSED = True + SUPPORT_COMPRESSED = False SUPPORT_SPARSE_DATA = False def __init__(self, filename): diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index 5976f2b265c..592fab813d1 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -1,30 +1,20 @@ import os.path -import pathlib -from AnyQt.QtWidgets import QFormLayout from AnyQt.QtCore import Qt +from AnyQt.QtGui import QBrush +from AnyQt.QtWidgets import QGridLayout from Orange.data.table import Table -from Orange.data.io import Compression, FileFormat, TabReader, CSVReader, PickleReader, \ - ExcelReader +from Orange.data.io import TabReader, CSVReader, PickleReader, ExcelReader from Orange.widgets import gui, widget from Orange.widgets.settings import Setting from Orange.widgets.utils import filedialogs from Orange.widgets.utils.widgetpreview import WidgetPreview from Orange.widgets.widget import Input -FILE_TYPES = [ - ("{} ({})".format(w.DESCRIPTION, w.EXTENSIONS[0]), - w.EXTENSIONS[0], - w.SUPPORT_SPARSE_DATA) - for w in (TabReader, CSVReader, PickleReader, ExcelReader) -] -COMPRESSIONS = [ - ("gzip ({})".format(Compression.GZIP), Compression.GZIP), - ("bzip2 ({})".format(Compression.BZIP2), Compression.BZIP2), - ("lzma ({})".format(Compression.XZ), Compression.XZ), -] +FILE_WRITERS = (TabReader, CSVReader, PickleReader, ExcelReader) +FILE_FORMATS = [f"{w.DESCRIPTION} ({w.EXTENSIONS[0]})" for w in FILE_WRITERS] class OWSave(widget.OWWidget): @@ -38,166 +28,145 @@ class Inputs: data = Input("Data", Table) class Error(widget.OWWidget.Error): - unsupported_extension = widget.Msg("Selected extension is not supported.") + unsupported_sparse = \ + widget.Msg("This file format does not support sparse data.") + general_error = widget.Msg("{}") want_main_area = False resizing_enabled = False + filetype = Setting(0) last_dir = Setting("") - auto_save = Setting(False) - filetype = Setting(FILE_TYPES[0][0]) - compression = Setting(COMPRESSIONS[0][0]) - compress = Setting(False) + compression = Setting(False) add_type_annotations = Setting(True) + auto_save = Setting(False) def __init__(self): super().__init__() self.data = None - self.filename = "" self.basename = "" - self.type_ext = "" - self.compress_ext = "" - - form = QFormLayout( - labelAlignment=Qt.AlignLeft, - formAlignment=Qt.AlignLeft, - rowWrapPolicy=QFormLayout.WrapLongRows, - verticalSpacing=10, - ) + self.dirty = False box = gui.vBox(self.controlArea, "Format") - gui.comboBox( - box, self, "filetype", - callback=self._update_text, - items=[item for item, _, _ in FILE_TYPES], - sendSelectedValue=True, - ) - form.addRow("File type", self.controls.filetype, ) - - gui.comboBox( - box, self, "compression", - callback=self._update_text, - items=[item for item, _ in COMPRESSIONS], - sendSelectedValue=True, - ) + box, self, "filetype", items=FILE_FORMATS, + callback=self._set_dirty, addSpace=2) + box2 = gui.indentedBox(box, sep=10) gui.checkBox( - box, self, "compress", label="Use compression", - callback=self._update_text, - ) - - form.addRow(self.controls.compress, self.controls.compression) - - box.layout().addLayout(form) - - self.annotations_cb = gui.checkBox( - None, self, "add_type_annotations", label="Add type annotations", - ) - form.addRow(self.annotations_cb, None) + box2, self, "compression", "Compress file (as gzip)", + callback=self._set_dirty, addSpace=2) + gui.checkBox( + box2, self, "add_type_annotations", label="Add type annotations", + callback=self._set_dirty) - self.save = gui.auto_commit( - self.controlArea, self, "auto_save", "Save", box=False, - commit=self.save_file, callback=self.adjust_label, - disabled=True, addSpace=True - ) + grid = QGridLayout() + gui.widgetBox(self.controlArea, box=True, orientation=grid) + self.save = gui.button(None, self, "Save", callback=self.save_file) self.save_as = gui.button( - self.controlArea, self, "Save As...", - callback=self.save_file_as, disabled=True - ) - self.save_as.setMinimumWidth(220) + None, self, "Save as ...", callback=self.save_file_as) + grid.addWidget(self.save, 0, 0) + grid.addWidget(self.save_as, 0, 1) + grid.setRowMinimumHeight(1, 8) + grid.addWidget( + gui.checkBox( + None, self, "auto_save", "Autosave when receiving new data"), + 2, 0, 2, 2) self.adjustSize() - - def get_writer_selected(self): - writer = FileFormat.get_reader(self.type_ext) - - ext = self.type_ext + self.compress_ext - if ext not in writer.EXTENSIONS: - self.Error.unsupported_extension() - return None - writer.EXTENSIONS = [ext] - return writer - - @classmethod - def remove_extensions(cls, filename): - if not filename: - return None - for ext in pathlib.PurePosixPath(filename).suffixes: - filename = filename.replace(ext, '') - return filename - - def adjust_label(self): - if self.filename: - text = "Auto save as '{}'" if self.auto_save else "Save as '{}'" - self.save.button.setText( - text.format(self.basename + self.type_ext + self.compress_ext)) + self._update_controls() @Inputs.data def dataset(self, data): + self.Error.clear() self.data = data - self.save.setDisabled(data is None) - self.save_as.setDisabled(data is None) - if data is None: - return - - items = [item for item, _, supports_sparse in FILE_TYPES - if supports_sparse or not data.is_sparse()] - if items != [self.controls.filetype.itemText(i) for i in - range(self.controls.filetype.count())]: - self.controls.filetype.clear() - self.controls.filetype.insertItems(0, items) - self.update_extension() - - self.save_file() + self.dirty = data is not None + self._update_combo_colors() + if self.auto_save and self.basename: + self.save_file() + self._update_controls() def save_file_as(self): - file_name = self.remove_extensions(self.filename) or os.path.join( - self.last_dir or os.path.expanduser("~"), - getattr(self.data, 'name', '')) - self.update_extension() - writer = self.get_writer_selected() - if not writer: - return - - filename, writer, _ = filedialogs.open_filename_dialog_save( - file_name, '', [writer], - ) + start_path = self.basename \ + or os.path.join(self.last_dir or os.path.expanduser("~"), + getattr(self.data, 'name', '')) + filename, _, _ = filedialogs.open_filename_dialog_save( + start_path, '', [FILE_WRITERS[self.filetype]]) if not filename: return - self.filename = filename - self.last_dir = os.path.split(self.filename)[0] - self.basename = os.path.basename(self.remove_extensions(filename)) - self.unconditional_save_file() - self.adjust_label() + self.last_dir, filename = os.path.split(filename) + self.basename, _ = os.path.splitext(filename) + self.save_file() + self._update_controls() def save_file(self): - if self.data is None: + writer = FILE_WRITERS[self.filetype] + if self.data is None \ + or self.data.is_sparse() and not writer.SUPPORT_SPARSE_DATA: return - if not self.filename: + if not self.basename: self.save_file_as() + return + try: + filename = os.path.join(self.last_dir, self._fullname) + writer.write(filename, self.data, self.add_type_annotations) + except Exception as err_value: + self.Error.general_error(str(err_value)) else: - try: - self.get_writer_selected().write( - os.path.join( - self.last_dir, - self.basename + self.type_ext + self.compress_ext), - self.data, self.add_type_annotations) - - except Exception as err_value: - self.error(str(err_value)) + self.Error.general_error.clear() + self.dirty = False + self._update_controls() + + @property + def _fullname(self): + writer = FILE_WRITERS[self.filetype] + return self.basename \ + + writer.EXTENSIONS[0] \ + + ".gzip" * (self.compression and writer.SUPPORT_COMPRESSED) + + def _update_combo_colors(self): + sparse_data = self.data is not None and self.data.is_sparse() + combo = self.controls.filetype + brushes = QBrush(Qt.gray), QBrush(Qt.black) + for i, writer in enumerate(FILE_WRITERS): + combo.setItemData( + i, brushes[not sparse_data or writer.SUPPORT_SPARSE_DATA], + Qt.TextColorRole) + + def _set_dirty(self): + self.dirty = True + self._update_controls() + + def _update_controls(self): + ctrl = self.controls + writer = FILE_WRITERS[self.filetype] + + self.Error.unsupported_sparse( + shown=self.data is not None + and self.data.is_sparse() + and not writer.SUPPORT_SPARSE_DATA) + ctrl.add_type_annotations.setEnabled(writer.OPTIONAL_TYPE_ANNOTATIONS) + ctrl.compression.setEnabled(writer.SUPPORT_COMPRESSED) + + self.save.setText( + f"Save as '{self._fullname}'" if self.basename else "Save") + enable_save = self.data is not None \ + and (not self.data.is_sparse() or writer.SUPPORT_SPARSE_DATA) + self.save.setVisible(self.dirty and bool(self.basename)) + self.save.setEnabled(enable_save) + self.save_as.setEnabled(enable_save) + ctrl.auto_save.setEnabled(enable_save and bool(self.basename)) + + @classmethod + def migrate_settings(cls, settings, version=0): + return + if version < 2: + for i, (compr, _) in enumerate(COMPRESSIONS): + if compr == settings["compression"]: + settings["compression"] = i + break 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(False) - if self.get_writer_selected().OPTIONAL_TYPE_ANNOTATIONS: - self.annotations_cb.setEnabled(True) - self.compress_ext = dict(COMPRESSIONS)[self.compression] if self.compress else '' - - def _update_text(self): - self.update_extension() - self.adjust_label() + settings["compression"] = 0 + settings["filetype"] = FILE_FORMATS.index(settings["filetype"]) if __name__ == "__main__": # pragma: no cover From 17efb735a1718c24d566cf374b1ee07f90a6ddbb Mon Sep 17 00:00:00 2001 From: janezd Date: Mon, 21 Jan 2019 00:00:28 +0100 Subject: [PATCH 02/12] OWSave: Improve GUI more --- Orange/widgets/data/owsave.py | 213 +++++++++++++++++----------------- 1 file changed, 107 insertions(+), 106 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index 592fab813d1..5124110650c 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -1,22 +1,16 @@ import os.path +from AnyQt.QtWidgets import QFileDialog from AnyQt.QtCore import Qt -from AnyQt.QtGui import QBrush -from AnyQt.QtWidgets import QGridLayout from Orange.data.table import Table from Orange.data.io import TabReader, CSVReader, PickleReader, ExcelReader from Orange.widgets import gui, widget from Orange.widgets.settings import Setting -from Orange.widgets.utils import filedialogs from Orange.widgets.utils.widgetpreview import WidgetPreview from Orange.widgets.widget import Input -FILE_WRITERS = (TabReader, CSVReader, PickleReader, ExcelReader) -FILE_FORMATS = [f"{w.DESCRIPTION} ({w.EXTENSIONS[0]})" for w in FILE_WRITERS] - - class OWSave(widget.OWWidget): name = "Save Data" description = "Save data to an output file." @@ -24,53 +18,60 @@ class OWSave(widget.OWWidget): category = "Data" keywords = [] + settings_version = 2 + + writers = [TabReader, CSVReader, PickleReader, ExcelReader] + filters = [f"{w.DESCRIPTION} ({w.EXTENSIONS[0]})" for w in writers] + filt_ext = {filter: w.EXTENSIONS[0] for filter, w in zip(filters, writers)} + userhome = os.path.expanduser(f"~{os.sep}") + class Inputs: data = Input("Data", Table) class Error(widget.OWWidget.Error): - unsupported_sparse = \ - widget.Msg("This file format does not support sparse data.") + # This message is short to (almost) fit into the widget's width + unsupported_sparse = widget.Msg("Format can't store sparse data.") + general_error = widget.Msg("{}") + + class Warning(widget.OWWidget.Warning): + no_file_name = widget.Msg("Set the file name.") general_error = widget.Msg("{}") want_main_area = False resizing_enabled = False + compress: bool + add_type_annotations: bool + filetype = Setting(0) last_dir = Setting("") - compression = Setting(False) + filter = Setting(filters[0]) + compress = Setting(False) add_type_annotations = Setting(True) auto_save = Setting(False) def __init__(self): super().__init__() self.data = None - self.basename = "" - self.dirty = False - - box = gui.vBox(self.controlArea, "Format") - gui.comboBox( - box, self, "filetype", items=FILE_FORMATS, - callback=self._set_dirty, addSpace=2) - box2 = gui.indentedBox(box, sep=10) + self.filename = "" + self.writer = self.writers[0] + + box = gui.vBox(self.controlArea, True) + box.layout().setSpacing(8) + self.lb_filename = gui.widgetLabel(box) gui.checkBox( - box2, self, "compression", "Compress file (as gzip)", - callback=self._set_dirty, addSpace=2) + box, self, "add_type_annotations", "Save with type annotations") gui.checkBox( - box2, self, "add_type_annotations", label="Add type annotations", - callback=self._set_dirty) - - grid = QGridLayout() - gui.widgetBox(self.controlArea, box=True, orientation=grid) - self.save = gui.button(None, self, "Save", callback=self.save_file) - self.save_as = gui.button( - None, self, "Save as ...", callback=self.save_file_as) - grid.addWidget(self.save, 0, 0) - grid.addWidget(self.save_as, 0, 1) - grid.setRowMinimumHeight(1, 8) - grid.addWidget( - gui.checkBox( - None, self, "auto_save", "Autosave when receiving new data"), - 2, 0, 2, 2) + box, self, "compress", "Compress file (gzip)") + self.bt_set_file = gui.button( + None, self, "Set File Name", callback=self.set_file_name) + box.layout().addWidget(self.bt_set_file, Qt.AlignRight) + + box = gui.vBox(self.controlArea, box=True) + box.layout().setSpacing(8) + gui.checkBox( + box, self, "auto_save", "Autosave when receiving new data") + self.bt_save = gui.button(box, self, "Save", callback=self.save_file) self.adjustSize() self._update_controls() @@ -78,95 +79,95 @@ def __init__(self): def dataset(self, data): self.Error.clear() self.data = data - self.dirty = data is not None - self._update_combo_colors() - if self.auto_save and self.basename: + if self.auto_save and self.filename: self.save_file() - self._update_controls() - def save_file_as(self): - start_path = self.basename \ - or os.path.join(self.last_dir or os.path.expanduser("~"), - getattr(self.data, 'name', '')) - filename, _, _ = filedialogs.open_filename_dialog_save( - start_path, '', [FILE_WRITERS[self.filetype]]) - if not filename: + self._update_controls() + if self.data is None: + self.info.set_input_summary(self.info.NoInput) + else: + self.info.set_input_summary( + str(len(self.data)), + f"Data set {self.data.name or '(no name)'} " + f"with {len(self.data)} instances") + + def set_file_name(self): + if self.filename: + start_dir = self.filename + else: + data_name = getattr(self.data, 'name', '') + if data_name: + data_name += self.filt_ext[self.filter] + start_dir = os.path.join(self.last_dir or self.userhome, data_name) + + dlg = QFileDialog(None, "Set File", start_dir, ";;".join(self.filters)) + dlg.setLabelText(dlg.Accept, "Select") + dlg.setAcceptMode(dlg.AcceptSave) + dlg.setSupportedSchemes(["file"]) + dlg.selectNameFilter(self.filter) + if dlg.exec() == dlg.Rejected: return - self.last_dir, filename = os.path.split(filename) - self.basename, _ = os.path.splitext(filename) - self.save_file() + self.filename = dlg.selectedFiles()[0] + self.last_dir = os.path.split(self.filename)[0] + self.filter = dlg.selectedNameFilter() + self.writer = self.writers[self.filters.index(self.filter)] self._update_controls() def save_file(self): - writer = FILE_WRITERS[self.filetype] - if self.data is None \ - or self.data.is_sparse() and not writer.SUPPORT_SPARSE_DATA: - return - if not self.basename: - self.save_file_as() + self.Error.general_error.clear() + if not self._can_save(): return + name = self.filename \ + + ".gz" * self.writer.SUPPORT_COMPRESSED * self.compress try: - filename = os.path.join(self.last_dir, self._fullname) - writer.write(filename, self.data, self.add_type_annotations) - except Exception as err_value: + self.writer.write(name, self.data, self.add_type_annotations) + except IOError as err_value: self.Error.general_error(str(err_value)) - else: - self.Error.general_error.clear() - self.dirty = False - self._update_controls() - - @property - def _fullname(self): - writer = FILE_WRITERS[self.filetype] - return self.basename \ - + writer.EXTENSIONS[0] \ - + ".gzip" * (self.compression and writer.SUPPORT_COMPRESSED) - - def _update_combo_colors(self): - sparse_data = self.data is not None and self.data.is_sparse() - combo = self.controls.filetype - brushes = QBrush(Qt.gray), QBrush(Qt.black) - for i, writer in enumerate(FILE_WRITERS): - combo.setItemData( - i, brushes[not sparse_data or writer.SUPPORT_SPARSE_DATA], - Qt.TextColorRole) - - def _set_dirty(self): - self.dirty = True - self._update_controls() def _update_controls(self): - ctrl = self.controls - writer = FILE_WRITERS[self.filetype] + has_file = bool(self.filename) + self.lb_filename.setVisible(has_file) + self.Warning.no_file_name(shown=not has_file) + if self.filename: + name = self.filename + if name.startswith(self.userhome): + name = name[len(self.userhome):] + self.lb_filename.setText(f"Save to: {name}") + + self.controls.add_type_annotations.setVisible( + has_file and self.writer.OPTIONAL_TYPE_ANNOTATIONS) + self.controls.compress.setVisible( + has_file and self.writer.SUPPORT_COMPRESSED) self.Error.unsupported_sparse( - shown=self.data is not None - and self.data.is_sparse() - and not writer.SUPPORT_SPARSE_DATA) - ctrl.add_type_annotations.setEnabled(writer.OPTIONAL_TYPE_ANNOTATIONS) - ctrl.compression.setEnabled(writer.SUPPORT_COMPRESSED) - - self.save.setText( - f"Save as '{self._fullname}'" if self.basename else "Save") - enable_save = self.data is not None \ - and (not self.data.is_sparse() or writer.SUPPORT_SPARSE_DATA) - self.save.setVisible(self.dirty and bool(self.basename)) - self.save.setEnabled(enable_save) - self.save_as.setEnabled(enable_save) - ctrl.auto_save.setEnabled(enable_save and bool(self.basename)) + shown=self.data is not None and self.data.is_sparse() + and self.filename + and not self.writer.SUPPORT_SPARSE_DATA) + self.bt_save.setEnabled(self._can_save()) + + def _can_save(self): + return self.data is not None \ + and bool(self.filename) \ + and (not self.data.is_sparse() or self.writer.SUPPORT_SPARSE_DATA) + + def send_report(self): + self.report_data_brief(self.data) + writer = self.writer + noyes = ["No", "Yes"] + self.report_items(( + ("File name", self.filename or "not set"), + ("Format", writer.DESCRIPTION), + ("Compression", writer.SUPPORT_COMPRESSED and noyes[self.compress]), + ("Type annotations", + writer.OPTIONAL_TYPE_ANNOTATIONS + and noyes[self.add_type_annotations]) + )) @classmethod def migrate_settings(cls, settings, version=0): - return if version < 2: - for i, (compr, _) in enumerate(COMPRESSIONS): - if compr == settings["compression"]: - settings["compression"] = i - break - else: - settings["compression"] = 0 - settings["filetype"] = FILE_FORMATS.index(settings["filetype"]) + settings["filter"] = settings.pop("filetype") if __name__ == "__main__": # pragma: no cover From 730223a3e1595c59dd0ac3dc172c9580c79666cd Mon Sep 17 00:00:00 2001 From: janezd Date: Wed, 23 Jan 2019 21:50:47 +0100 Subject: [PATCH 03/12] OWSave: Add tests --- Orange/widgets/data/tests/test_owsave.py | 429 +++++++++++++++++------ 1 file changed, 318 insertions(+), 111 deletions(-) diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index d37a3239e28..eef7787381f 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -1,145 +1,352 @@ -# Test methods with long descriptive names can omit docstrings -# pylint: disable=missing-docstring +# pylint: disable=missing-docstring, protected-access +import unittest from unittest.mock import patch, Mock -import itertools +import os + +from AnyQt.QtWidgets import QDialog + +import scipy.sparse as sp from Orange.data import Table -from Orange.data.io import Compression, CSVReader, TabReader, PickleReader, \ - ExcelReader, FileFormat +from Orange.data.io import CSVReader, TabReader, PickleReader, ExcelReader from Orange.tests import named_file from Orange.widgets.tests.base import WidgetTest -from Orange.widgets.utils.filedialogs import format_filter from Orange.widgets.data.owsave import OWSave -FILE_TYPES = [ - ("{} ({})".format(w.DESCRIPTION, w.EXTENSIONS[0]), - w.EXTENSIONS[0], - w.SUPPORT_SPARSE_DATA) - for w in (TabReader, CSVReader, PickleReader, ExcelReader) -] - -COMPRESSIONS = [ - ("gzip ({})".format(Compression.GZIP), Compression.GZIP), - ("bzip2 ({})".format(Compression.BZIP2), Compression.BZIP2), - ("lzma ({})".format(Compression.XZ), Compression.XZ), -] +FILE_TYPES = [("{} ({})".format(w.DESCRIPTION, w.EXTENSIONS[0]), w) + for w in (TabReader, CSVReader, PickleReader, ExcelReader)] -class AddedFormat(FileFormat): - EXTENSIONS = ('.234',) - DESCRIPTION = "Test if a dialog format works after reading OWSave" - def write_file(self): - pass +# Yay, MS Windows! +# This is not the proper general way to do it, but it's simplest and sufficient +# Short name is suitable for the functions, purpose +def _w(s): # pylint: disable=invalid-name + return s.replace("/", os.sep) class TestOWSave(WidgetTest): - def setUp(self): self.widget = self.create_widget(OWSave) # type: OWSave + self.iris = Table("iris") - def test_writer(self): - compressions = [self.widget.controls.compression.itemText(i) for i in - range(self.widget.controls.compression.count())] - types = [self.widget.controls.filetype.itemText(i) - for i in range(self.widget.controls.filetype.count())] - for t, c, d in itertools.product(types, compressions, [True, False]): - self.widget.compression = c - self.widget.compress = d - self.widget.filetype = t - self.widget.update_extension() - self.assertEqual( - len(self.widget.get_writer_selected().EXTENSIONS), 1) - - def test_ordinary_save(self): - self.send_signal(self.widget.Inputs.data, Table("iris")) - - for ext, suffix, _ in FILE_TYPES: - self.widget.filetype = ext - self.widget.update_extension() - writer = self.widget.get_writer_selected() - with named_file("", suffix=suffix) as filename: - def choose_file(a, b, c, d, e, fn=filename, w=writer): - return fn, format_filter(w) - - with patch("AnyQt.QtWidgets.QFileDialog.getSaveFileName", choose_file): - self.widget.save_file_as() - self.assertEqual(len(Table(filename)), 150) + def test_dataset(self): + widget = self.widget + insum = widget.info.set_input_summary = Mock() + savefile = widget.save_file = Mock() + + datasig = widget.Inputs.data + self.send_signal(datasig, self.iris) + self.assertEqual(insum.call_args[0][0], "150") + self.assertFalse(widget.bt_save.isEnabled()) + insum.reset_mock() + savefile.reset_mock() + + widget.filename = "foo.tab" + widget.writer = TabReader + widget.auto_save = False + self.send_signal(datasig, self.iris) + self.assertEqual(insum.call_args[0][0], "150") + self.assertTrue(widget.bt_save.isEnabled()) + savefile.assert_not_called() + + widget.auto_save = True + self.send_signal(datasig, self.iris) + self.assertEqual(insum.call_args[0][0], "150") + self.assertTrue(widget.bt_save.isEnabled()) + savefile.assert_called() + + self.send_signal(datasig, None) + insum.assert_called_with(widget.info.NoInput) + self.assertFalse(widget.bt_save.isEnabled()) + + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_set_file_name_start_dir(self, filedialog): + widget = self.widget + + dlginst = filedialog.return_value + dlginst.exec.return_value = dlginst.Rejected = QDialog.Rejected + + widget.filename = _w("/usr/foo/bar.csv") + widget.filter = FILE_TYPES[1][0] + widget.set_file_name() + self.assertEqual(filedialog.call_args[0][2], widget.filename) + + widget.filename = "" + widget.last_dir = _w("/usr/bar") + widget.set_file_name() + self.assertEqual(filedialog.call_args[0][2], _w("/usr/bar/")) + + self.send_signal(widget.Inputs.data, self.iris) + widget.last_dir = _w("/usr/bar") + widget.set_file_name() + self.assertEqual(filedialog.call_args[0][2], _w("/usr/bar/iris.csv")) + + widget.last_dir = "" + widget.set_file_name() + self.assertEqual(filedialog.call_args[0][2], + os.path.expanduser(_w("~/iris.csv"))) + + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_set_file_name(self, filedialog): + widget = self.widget + widget.filename = _w("/usr/foo/bar.csv") + widget.last_dir = _w("/usr/foo/") + widget.writer = widget.writers[1] + widget.filter = FILE_TYPES[1][0] + + widget._update_controls = Mock() + + dlginst = filedialog.return_value + dlginst.selectedFiles.return_value = [_w("/bar/baz.csv")] + dlginst.selectedNameFilter.return_value = FILE_TYPES[0][0] + + dlginst.exec.return_value = dlginst.Rejected = QDialog.Rejected + widget.set_file_name() + self.assertEqual(widget.filename, _w("/usr/foo/bar.csv")) + self.assertEqual(widget.last_dir, _w("/usr/foo/")) + self.assertEqual(widget.filter, FILE_TYPES[1][0]) + self.assertIs(widget.writer, widget.writers[1]) + widget._update_controls.assert_not_called() + + dlginst.exec.return_value = dlginst.Accepted = QDialog.Accepted + widget.set_file_name() + self.assertEqual(widget.filename, _w("/bar/baz.csv")) + self.assertEqual(widget.last_dir, _w("/bar")) + self.assertEqual(widget.filter, FILE_TYPES[0][0]) + self.assertIs(widget.writer, widget.writers[0]) + widget._update_controls.assert_called() + + def set_mock_writer(self): + widget = self.widget + writer = widget.writer = Mock() + writer.write = Mock() + writer.SUPPORT_COMPRESSED = True + writer.SUPPORT_SPARSE_DATA = False + writer.OPTIONAL_TYPE_ANNOTATIONS = False + + def test_save_file_check_can_save(self): + widget = self.widget + self.set_mock_writer() + + widget.save_file() + widget.writer.write.assert_not_called() + + widget.filename = "foo" + widget.save_file() + widget.writer.write.assert_not_called() + + widget.filename = "" + self.send_signal(widget.Inputs.data, self.iris) + widget.save_file() + widget.writer.write.assert_not_called() + + widget.filename = "foo" + widget.save_file() + widget.writer.write.assert_called() + widget.writer.reset_mock() + + self.iris.X = sp.csr_matrix(self.iris.X) + widget.save_file() + widget.writer.write.assert_not_called() + + widget.writer.SUPPORT_SPARSE_DATA = True + widget.save_file() + widget.writer.write.assert_called() - @patch('Orange.data.io.FileFormat.write') - def test_annotations(self, write): + def test_save_file_write_errors(self): widget = self.widget + datasig = widget.Inputs.data - self.send_signal(widget.Inputs.data, Table("iris")) - widget.filetype = FILE_TYPES[1][0] - widget.filename = 'foo.csv' - widget.update_extension() + widget.auto_save = True + widget.filename = _w("bar/foo") + self.set_mock_writer() - widget.add_type_annotations = False - widget.unconditional_save_file() - write.assert_called() - self.assertFalse(write.call_args[0][2]) + widget.writer.write.side_effect = IOError + self.send_signal(datasig, self.iris) + self.assertTrue(widget.Error.general_error.is_shown()) + widget.writer.write.side_effect = None + self.send_signal(datasig, self.iris) + self.assertFalse(widget.Error.general_error.is_shown()) + + widget.writer.write.side_effect = IOError + self.send_signal(datasig, self.iris) + self.assertTrue(widget.Error.general_error.is_shown()) + + widget.writer.write.side_effect = None + self.send_signal(datasig, None) + self.assertFalse(widget.Error.general_error.is_shown()) + + widget.writer.write.side_effect = ValueError + self.assertRaises(ValueError, self.send_signal, datasig, self.iris) + + def test_save_file_write(self): + widget = self.widget + datasig = widget.Inputs.data + + self.set_mock_writer() + widget.auto_save = True + + widget.filename = _w("bar/foo.csv") + widget.compress = False widget.add_type_annotations = True - widget.unconditional_save_file() - self.assertTrue(write.call_args[0][2]) + self.send_signal(datasig, self.iris) + widget.writer.write.assert_called_with( + _w("bar/foo.csv"), self.iris, True) + + widget.compress = True + self.send_signal(datasig, self.iris) + widget.writer.write.assert_called_with( + _w("bar/foo.csv.gz"), self.iris, True) - def test_disable_checkbox(self): + widget.writer.SUPPORT_COMPRESSED = False + self.send_signal(datasig, self.iris) + widget.writer.write.assert_called_with( + _w("bar/foo.csv"), self.iris, True) + + def test_file_label(self): widget = self.widget - for type_ in FILE_TYPES: - widget.filetype = type_[0] - widget.update_extension() - if widget.get_writer_selected().OPTIONAL_TYPE_ANNOTATIONS: - self.assertTrue(widget.annotations_cb.isEnabled()) - else: - self.assertFalse(widget.annotations_cb.isEnabled()) - - def test_compression(self): - self.send_signal(self.widget.Inputs.data, Table("iris")) - - self.widget.compress = True - for type, compression in itertools.product([[x, ext] for x, ext, _ in FILE_TYPES], - COMPRESSIONS): - self.widget.filetype = type[0] - self.widget.compression = compression[0] - self.widget.update_extension() - writer = self.widget.get_writer_selected() - with named_file("", - suffix=type[1] + compression[1]) as filename: - def choose_file(a, b, c, d, e, fn=filename, w=writer): - return fn, format_filter(w) - - with patch("AnyQt.QtWidgets.QFileDialog.getSaveFileName", choose_file): - self.widget.save_file_as() - self.assertEqual(len(Table(filename)), 150) - def test_format_combo(self): + widget.filename = "" + widget._update_controls() + self.assertTrue(widget.lb_filename.isHidden()) + self.assertTrue(widget.Warning.no_file_name.is_shown()) + + widget.filename = _w("/foo/bar/baz.csv") + widget._update_controls() + self.assertFalse(widget.lb_filename.isHidden()) + self.assertIn( + widget.lb_filename.text(), _w("Save to: /foo/bar/baz.csv")) + self.assertFalse(widget.Warning.no_file_name.is_shown()) + + widget.filename = os.path.expanduser(_w("~/baz/bar/foo.csv")) + widget._update_controls() + self.assertFalse(widget.lb_filename.isHidden()) + self.assertEqual( + widget.lb_filename.text(), _w("Save to: baz/bar/foo.csv")) + + def test_annotation_checkbox(self): + widget = self.widget + for _, widget.writer in FILE_TYPES: + widget.filename = f"foo.{widget.writer.EXTENSIONS[0]}" + widget._update_controls() + self.assertIsNot(widget.controls.add_type_annotations.isHidden(), + widget.writer.OPTIONAL_TYPE_ANNOTATIONS, + msg=f"for {widget.writer}") + self.assertIsNot(widget.controls.compress.isHidden(), + widget.writer.SUPPORT_COMPRESSED, + msg=f"for {widget.writer}") + + widget.writer = TabReader + widget.filename = "" + self.widget._update_controls() + self.assertFalse(widget.controls.add_type_annotations.isVisible()) + self.assertFalse(widget.controls.compress.isVisible()) + + def test_sparse_error(self): widget = self.widget - filetype = widget.controls.filetype + err = widget.Error.unsupported_sparse - widget.save_file = Mock() + widget.writer = ExcelReader + widget.filename = "foo.xlsx" + widget.data = self.iris - data = Table("iris") - sparse_data = Table("iris") - sparse_data.is_sparse = Mock(return_value=True) + widget._update_controls() + self.assertFalse(err.is_shown()) - self.send_signal(widget.Inputs.data, data) - n_nonsparse = filetype.count() + widget.data.X = sp.csr_matrix(widget.data.X) + widget._update_controls() + self.assertTrue(err.is_shown()) - self.send_signal(widget.Inputs.data, sparse_data) - n_sparse = filetype.count() - self.assertGreater(n_nonsparse, n_sparse) + widget.writer = PickleReader + widget._update_controls() + self.assertFalse(err.is_shown()) + + widget.writer = ExcelReader + widget._update_controls() + self.assertTrue(err.is_shown()) + + widget.data = None + widget._update_controls() + self.assertFalse(err.is_shown()) + + def test_send_report(self): + widget = self.widget + + widget.report_items = Mock() + for _, writer in FILE_TYPES: + widget.writer = writer + for widget.compress in (False, True): + for widget.add_type_annotations in (False, True): + widget.filename = f"foo.{writer.EXTENSIONS[0]}" + widget.send_report() + items = dict(widget.report_items.call_args[0][0]) + msg = f"for {widget.writer}, " \ + f"annotations={widget.add_type_annotations}, " \ + f"compress={widget.compress}" + self.assertEqual(items["File name"], + f"foo.{writer.EXTENSIONS[0]}", msg=msg) + if writer.SUPPORT_COMPRESSED: + self.assertEqual( + items["Compression"], + ["No", "Yes"][widget.compress], + msg=msg) + else: + self.assertFalse(items["Compression"], msg=msg) + if writer.OPTIONAL_TYPE_ANNOTATIONS: + self.assertEqual( + items["Type annotations"], + ["No", "Yes"][widget.add_type_annotations], + msg=msg) + else: + self.assertFalse(items["Type annotations"], msg=msg) + + +class TestFunctionalOWSave(WidgetTest): + def setUp(self): + self.widget = self.create_widget(OWSave) # type: OWSave + self.iris = Table("iris") + + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_save_uncompressed(self, filedialog): + widget = self.widget + widget.auto_save = False + + dlg = filedialog.return_value + dlg.exec.return_value = dlg.Accepted = QDialog.Accepted + dlg = filedialog.return_value + + spiris = Table("iris") + spiris.X = sp.csr_matrix(spiris.X) + + for dlg.selectedNameFilter.return_value, writer in FILE_TYPES: + widget.write = writer + ext = writer.EXTENSIONS[0] + with named_file("", suffix=ext) as filename: + dlg.selectedFiles.return_value = [filename] + + self.send_signal(widget.Inputs.data, self.iris) + widget.bt_set_file.click() + widget.bt_save.click() + self.assertEqual(len(Table(filename)), 150) - self.send_signal(widget.Inputs.data, sparse_data) - self.assertEqual(filetype.count(), n_sparse) + if writer.SUPPORT_SPARSE_DATA: + self.send_signal(widget.Inputs.data, spiris) + widget.bt_set_file.click() + widget.bt_save.click() + self.assertEqual(len(Table(filename)), 150) - self.send_signal(widget.Inputs.data, data) - self.assertEqual(filetype.count(), n_nonsparse) + if writer.SUPPORT_COMPRESSED: + with named_file("", suffix=ext + ".gz") as filename: + widget.compress = True + dlg.selectedFiles.return_value = [filename[:-3]] + self.send_signal(widget.Inputs.data, self.iris) + widget.bt_set_file.click() + widget.bt_save.click() + self.assertEqual(len(Table(filename)), 150) + widget.compress = False - self.send_signal(widget.Inputs.data, None) - self.send_signal(widget.Inputs.data, data) - self.assertEqual(filetype.count(), n_nonsparse) - self.send_signal(widget.Inputs.data, None) - self.send_signal(widget.Inputs.data, sparse_data) - self.assertEqual(filetype.count(), n_sparse) +if __name__ == "__main__": + unittest.main() From 990a94254aa8714734dda18fab5c7d14e2a9fa0e Mon Sep 17 00:00:00 2001 From: janezd Date: Thu, 31 Jan 2019 23:40:14 +0100 Subject: [PATCH 04/12] OWSave: Change GUI as decided last week --- Orange/widgets/data/owsave.py | 133 +++++++++++--------- Orange/widgets/data/tests/test_owsave.py | 147 +++++++++++------------ 2 files changed, 145 insertions(+), 135 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index 5124110650c..d3d0d784cfd 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -1,7 +1,6 @@ import os.path -from AnyQt.QtWidgets import QFileDialog -from AnyQt.QtCore import Qt +from AnyQt.QtWidgets import QFileDialog, QGridLayout, QWidget from Orange.data.table import Table from Orange.data.io import TabReader, CSVReader, PickleReader, ExcelReader @@ -21,7 +20,7 @@ class OWSave(widget.OWWidget): settings_version = 2 writers = [TabReader, CSVReader, PickleReader, ExcelReader] - filters = [f"{w.DESCRIPTION} ({w.EXTENSIONS[0]})" for w in writers] + filters = [f"{w.DESCRIPTION} (*{w.EXTENSIONS[0]})" for w in writers] filt_ext = {filter: w.EXTENSIONS[0] for filter, w in zip(filters, writers)} userhome = os.path.expanduser(f"~{os.sep}") @@ -29,13 +28,12 @@ class Inputs: data = Input("Data", Table) class Error(widget.OWWidget.Error): - # This message is short to (almost) fit into the widget's width - unsupported_sparse = widget.Msg("Format can't store sparse data.") + unsupported_sparse = widget.Msg("Use .pkl format for sparse data.") + no_file_name = widget.Msg("File name is not set.") general_error = widget.Msg("{}") class Warning(widget.OWWidget.Warning): - no_file_name = widget.Msg("Set the file name.") - general_error = widget.Msg("{}") + ignored_flag = widget.Msg("{} ignored for this format.") want_main_area = False resizing_enabled = False @@ -43,7 +41,6 @@ class Warning(widget.OWWidget.Warning): compress: bool add_type_annotations: bool - filetype = Setting(0) last_dir = Setting("") filter = Setting(filters[0]) compress = Setting(False) @@ -56,22 +53,31 @@ def __init__(self): self.filename = "" self.writer = self.writers[0] - box = gui.vBox(self.controlArea, True) - box.layout().setSpacing(8) - self.lb_filename = gui.widgetLabel(box) - gui.checkBox( - box, self, "add_type_annotations", "Save with type annotations") - gui.checkBox( - box, self, "compress", "Compress file (gzip)") - self.bt_set_file = gui.button( - None, self, "Set File Name", callback=self.set_file_name) - box.layout().addWidget(self.bt_set_file, Qt.AlignRight) - - box = gui.vBox(self.controlArea, box=True) - box.layout().setSpacing(8) - gui.checkBox( - box, self, "auto_save", "Autosave when receiving new data") - self.bt_save = gui.button(box, self, "Save", callback=self.save_file) + grid = QGridLayout() + gui.widgetBox(self.controlArea, box=True, orientation=grid) + grid.setSpacing(8) + self.bt_save = gui.button(None, self, "Save", callback=self.save_file) + grid.addWidget(self.bt_save, 0, 0) + grid.addWidget( + gui.button(None, self, "Save as ...", callback=self.save_file_as), + 0, 1) + grid.addWidget( + gui.checkBox(None, self, "auto_save", + "Autosave when receiving new data"), + 1, 0, 1, 2) + grid.addWidget(QWidget(), 2, 0, 1, 2) + + grid.addWidget( + gui.checkBox( + None, self, "add_type_annotations", + "Save with type annotations", callback=self._update_controls), + 3, 0, 1, 2) + grid.addWidget( + gui.checkBox( + None, self, "compress", "Compress file (gzip)", + callback=self._update_controls), + 4, 0, 1, 2) + self.adjustSize() self._update_controls() @@ -79,8 +85,6 @@ def __init__(self): def dataset(self, data): self.Error.clear() self.data = data - if self.auto_save and self.filename: - self.save_file() self._update_controls() if self.data is None: @@ -91,7 +95,10 @@ def dataset(self, data): f"Data set {self.data.name or '(no name)'} " f"with {len(self.data)} instances") - def set_file_name(self): + if self.auto_save and self.filename: + self.save_file() + + def save_file_as(self): if self.filename: start_dir = self.filename else: @@ -100,56 +107,68 @@ def set_file_name(self): data_name += self.filt_ext[self.filter] start_dir = os.path.join(self.last_dir or self.userhome, data_name) - dlg = QFileDialog(None, "Set File", start_dir, ";;".join(self.filters)) - dlg.setLabelText(dlg.Accept, "Select") - dlg.setAcceptMode(dlg.AcceptSave) - dlg.setSupportedSchemes(["file"]) - dlg.selectNameFilter(self.filter) - if dlg.exec() == dlg.Rejected: + filename, selected_filter = QFileDialog.getSaveFileName( + self, "Save data", start_dir, ";;".join(self.filters), self.filter) + if not filename: return - self.filename = dlg.selectedFiles()[0] - self.last_dir = os.path.split(self.filename)[0] - self.filter = dlg.selectedNameFilter() + self.filename = filename + self.last_dir = os.path.split(filename)[0] + self.filter = selected_filter self.writer = self.writers[self.filters.index(self.filter)] self._update_controls() + self.save_file() def save_file(self): + if not self.filename: + self.save_file_as() + return self.Error.general_error.clear() if not self._can_save(): return - name = self.filename \ - + ".gz" * self.writer.SUPPORT_COMPRESSED * self.compress try: - self.writer.write(name, self.data, self.add_type_annotations) + self.writer.write( + self._fullname(), self.data, self.add_type_annotations) except IOError as err_value: self.Error.general_error(str(err_value)) + def _fullname(self): + return self.filename \ + + ".gz" * self.writer.SUPPORT_COMPRESSED * self.compress + def _update_controls(self): - has_file = bool(self.filename) - self.lb_filename.setVisible(has_file) - self.Warning.no_file_name(shown=not has_file) if self.filename: - name = self.filename - if name.startswith(self.userhome): - name = name[len(self.userhome):] - self.lb_filename.setText(f"Save to: {name}") - - self.controls.add_type_annotations.setVisible( - has_file and self.writer.OPTIONAL_TYPE_ANNOTATIONS) - self.controls.compress.setVisible( - has_file and self.writer.SUPPORT_COMPRESSED) + self.bt_save.setText( + f"Save as {os.path.split(self._fullname())[1]}") + else: + self.bt_save.setText("Save") + self.Error.no_file_name(shown=not self.filename) self.Error.unsupported_sparse( shown=self.data is not None and self.data.is_sparse() - and self.filename - and not self.writer.SUPPORT_SPARSE_DATA) - self.bt_save.setEnabled(self._can_save()) + and self.filename and not self.writer.SUPPORT_SPARSE_DATA) + + if self.data is None or not self.filename: + self.Warning.ignored_flag.clear() + else: + no_compress = self.compress \ + and not self.writer.SUPPORT_COMPRESSED + no_anotation = self.add_type_annotations \ + and not self.writer.OPTIONAL_TYPE_ANNOTATIONS + ignored = [ + "", + "Compression flag is", + "Type annotation flag is", + "Compression and type annotation flags are" + ][no_compress + 2 * no_anotation] + self.Warning.ignored_flag(ignored, shown=bool(ignored)) def _can_save(self): - return self.data is not None \ - and bool(self.filename) \ - and (not self.data.is_sparse() or self.writer.SUPPORT_SPARSE_DATA) + return not ( + self.data is None + or not self.filename + or self.data.is_sparse() and not self.writer.SUPPORT_SPARSE_DATA + ) def send_report(self): self.report_data_brief(self.data) diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index eef7787381f..9e1ce5fdb83 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -3,8 +3,6 @@ from unittest.mock import patch, Mock import os -from AnyQt.QtWidgets import QDialog - import scipy.sparse as sp from Orange.data import Table @@ -14,13 +12,13 @@ from Orange.widgets.data.owsave import OWSave -FILE_TYPES = [("{} ({})".format(w.DESCRIPTION, w.EXTENSIONS[0]), w) +FILE_TYPES = [("{} (*{})".format(w.DESCRIPTION, w.EXTENSIONS[0]), w) for w in (TabReader, CSVReader, PickleReader, ExcelReader)] # Yay, MS Windows! # This is not the proper general way to do it, but it's simplest and sufficient -# Short name is suitable for the functions, purpose +# Short name is suitable for the function's purpose def _w(s): # pylint: disable=invalid-name return s.replace("/", os.sep) @@ -38,7 +36,6 @@ def test_dataset(self): datasig = widget.Inputs.data self.send_signal(datasig, self.iris) self.assertEqual(insum.call_args[0][0], "150") - self.assertFalse(widget.bt_save.isEnabled()) insum.reset_mock() savefile.reset_mock() @@ -47,48 +44,44 @@ def test_dataset(self): widget.auto_save = False self.send_signal(datasig, self.iris) self.assertEqual(insum.call_args[0][0], "150") - self.assertTrue(widget.bt_save.isEnabled()) savefile.assert_not_called() widget.auto_save = True self.send_signal(datasig, self.iris) self.assertEqual(insum.call_args[0][0], "150") - self.assertTrue(widget.bt_save.isEnabled()) savefile.assert_called() self.send_signal(datasig, None) insum.assert_called_with(widget.info.NoInput) - self.assertFalse(widget.bt_save.isEnabled()) - @patch("Orange.widgets.data.owsave.QFileDialog") - def test_set_file_name_start_dir(self, filedialog): + @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") + def test_save_file_as_start_dir(self, filedialog): widget = self.widget - - dlginst = filedialog.return_value - dlginst.exec.return_value = dlginst.Rejected = QDialog.Rejected - widget.filename = _w("/usr/foo/bar.csv") widget.filter = FILE_TYPES[1][0] - widget.set_file_name() + + filedialog.return_value = "", FILE_TYPES[0][0] + + widget.save_file_as() self.assertEqual(filedialog.call_args[0][2], widget.filename) widget.filename = "" widget.last_dir = _w("/usr/bar") - widget.set_file_name() + widget.save_file_as() self.assertEqual(filedialog.call_args[0][2], _w("/usr/bar/")) self.send_signal(widget.Inputs.data, self.iris) widget.last_dir = _w("/usr/bar") - widget.set_file_name() + widget.save_file_as() self.assertEqual(filedialog.call_args[0][2], _w("/usr/bar/iris.csv")) widget.last_dir = "" - widget.set_file_name() + widget.save_file_as() self.assertEqual(filedialog.call_args[0][2], os.path.expanduser(_w("~/iris.csv"))) - @patch("Orange.widgets.data.owsave.QFileDialog") - def test_set_file_name(self, filedialog): + @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") + def test_save_file_as_name(self, filedialog): widget = self.widget widget.filename = _w("/usr/foo/bar.csv") widget.last_dir = _w("/usr/foo/") @@ -96,26 +89,25 @@ def test_set_file_name(self, filedialog): widget.filter = FILE_TYPES[1][0] widget._update_controls = Mock() + widget.save_file = Mock() - dlginst = filedialog.return_value - dlginst.selectedFiles.return_value = [_w("/bar/baz.csv")] - dlginst.selectedNameFilter.return_value = FILE_TYPES[0][0] - - dlginst.exec.return_value = dlginst.Rejected = QDialog.Rejected - widget.set_file_name() + filedialog.return_value = "", FILE_TYPES[0][0] + widget.save_file_as() self.assertEqual(widget.filename, _w("/usr/foo/bar.csv")) self.assertEqual(widget.last_dir, _w("/usr/foo/")) self.assertEqual(widget.filter, FILE_TYPES[1][0]) self.assertIs(widget.writer, widget.writers[1]) widget._update_controls.assert_not_called() + widget.save_file.assert_not_called() - dlginst.exec.return_value = dlginst.Accepted = QDialog.Accepted - widget.set_file_name() + filedialog.return_value = _w("/bar/baz.csv"), FILE_TYPES[0][0] + widget.save_file_as() self.assertEqual(widget.filename, _w("/bar/baz.csv")) self.assertEqual(widget.last_dir, _w("/bar")) self.assertEqual(widget.filter, FILE_TYPES[0][0]) self.assertIs(widget.writer, widget.writers[0]) widget._update_controls.assert_called() + widget.save_file.assert_called() def set_mock_writer(self): widget = self.widget @@ -127,6 +119,7 @@ def set_mock_writer(self): def test_save_file_check_can_save(self): widget = self.widget + widget.save_file_as = Mock(return_value=("", 0)) self.set_mock_writer() widget.save_file() @@ -200,49 +193,17 @@ def test_save_file_write(self): widget.writer.write.assert_called_with( _w("bar/foo.csv.gz"), self.iris, True) - widget.writer.SUPPORT_COMPRESSED = False - self.send_signal(datasig, self.iris) - widget.writer.write.assert_called_with( - _w("bar/foo.csv"), self.iris, True) - - def test_file_label(self): + def test_file_name_label(self): widget = self.widget widget.filename = "" widget._update_controls() - self.assertTrue(widget.lb_filename.isHidden()) - self.assertTrue(widget.Warning.no_file_name.is_shown()) + self.assertTrue(widget.Error.no_file_name.is_shown()) widget.filename = _w("/foo/bar/baz.csv") widget._update_controls() - self.assertFalse(widget.lb_filename.isHidden()) - self.assertIn( - widget.lb_filename.text(), _w("Save to: /foo/bar/baz.csv")) - self.assertFalse(widget.Warning.no_file_name.is_shown()) - - widget.filename = os.path.expanduser(_w("~/baz/bar/foo.csv")) - widget._update_controls() - self.assertFalse(widget.lb_filename.isHidden()) - self.assertEqual( - widget.lb_filename.text(), _w("Save to: baz/bar/foo.csv")) - - def test_annotation_checkbox(self): - widget = self.widget - for _, widget.writer in FILE_TYPES: - widget.filename = f"foo.{widget.writer.EXTENSIONS[0]}" - widget._update_controls() - self.assertIsNot(widget.controls.add_type_annotations.isHidden(), - widget.writer.OPTIONAL_TYPE_ANNOTATIONS, - msg=f"for {widget.writer}") - self.assertIsNot(widget.controls.compress.isHidden(), - widget.writer.SUPPORT_COMPRESSED, - msg=f"for {widget.writer}") - - widget.writer = TabReader - widget.filename = "" - self.widget._update_controls() - self.assertFalse(widget.controls.add_type_annotations.isVisible()) - self.assertFalse(widget.controls.compress.isVisible()) + self.assertFalse(widget.Error.no_file_name.is_shown()) + self.assertIn("baz.csv", widget.bt_save.text()) def test_sparse_error(self): widget = self.widget @@ -271,6 +232,43 @@ def test_sparse_error(self): widget._update_controls() self.assertFalse(err.is_shown()) + def test_ignored_flags_warnings(self): + widget = self.widget + + widget.writer = ExcelReader + widget.compress = True + widget.add_type_annotations = True + widget._update_controls() + self.assertFalse(widget.Warning.ignored_flag.is_shown()) + + widget.filename = "test.xlsx" + widget._update_controls() + self.assertFalse(widget.Warning.ignored_flag.is_shown()) + + self.send_signal(widget.Inputs.data, self.iris) + widget._update_controls() + self.assertTrue(widget.Warning.ignored_flag.is_shown()) + + widget.compress = False + widget._update_controls() + self.assertTrue(widget.Warning.ignored_flag.is_shown()) + + widget.add_type_annotations = False + widget._update_controls() + self.assertFalse(widget.Warning.ignored_flag.is_shown()) + + widget.writer = PickleReader + widget.filename = "test.pkl" + widget.add_type_annotations = True + widget.compress = False + widget._update_controls() + self.assertTrue(widget.Warning.ignored_flag.is_shown()) + + widget.add_type_annotations = False + widget.compress = True + widget._update_controls() + self.assertFalse(widget.Warning.ignored_flag.is_shown()) + def test_send_report(self): widget = self.widget @@ -308,42 +306,35 @@ def setUp(self): self.widget = self.create_widget(OWSave) # type: OWSave self.iris = Table("iris") - @patch("Orange.widgets.data.owsave.QFileDialog") + @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") def test_save_uncompressed(self, filedialog): widget = self.widget widget.auto_save = False - dlg = filedialog.return_value - dlg.exec.return_value = dlg.Accepted = QDialog.Accepted - dlg = filedialog.return_value - spiris = Table("iris") spiris.X = sp.csr_matrix(spiris.X) - for dlg.selectedNameFilter.return_value, writer in FILE_TYPES: + for selected_filter, writer in FILE_TYPES: widget.write = writer ext = writer.EXTENSIONS[0] with named_file("", suffix=ext) as filename: - dlg.selectedFiles.return_value = [filename] + filedialog.return_value = filename, selected_filter self.send_signal(widget.Inputs.data, self.iris) - widget.bt_set_file.click() - widget.bt_save.click() + widget.save_file_as() self.assertEqual(len(Table(filename)), 150) if writer.SUPPORT_SPARSE_DATA: self.send_signal(widget.Inputs.data, spiris) - widget.bt_set_file.click() - widget.bt_save.click() + widget.save_file() self.assertEqual(len(Table(filename)), 150) if writer.SUPPORT_COMPRESSED: with named_file("", suffix=ext + ".gz") as filename: + filedialog.return_value = filename[:-3], selected_filter widget.compress = True - dlg.selectedFiles.return_value = [filename[:-3]] self.send_signal(widget.Inputs.data, self.iris) - widget.bt_set_file.click() - widget.bt_save.click() + widget.save_file_as() self.assertEqual(len(Table(filename)), 150) widget.compress = False From 144d1c2ff6c0f584a896b897d0c1cf360b2884c5 Mon Sep 17 00:00:00 2001 From: janezd Date: Fri, 15 Feb 2019 16:34:38 +0100 Subject: [PATCH 05/12] OWSave: Change GUI as decided next last week --- Orange/widgets/data/owsave.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index d3d0d784cfd..28a9b1238d1 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -63,7 +63,8 @@ def __init__(self): 0, 1) grid.addWidget( gui.checkBox(None, self, "auto_save", - "Autosave when receiving new data"), + "Autosave when receiving new data", + callback=self._update_controls), 1, 0, 1, 2) grid.addWidget(QWidget(), 2, 0, 1, 2) @@ -142,7 +143,7 @@ def _update_controls(self): f"Save as {os.path.split(self._fullname())[1]}") else: self.bt_save.setText("Save") - self.Error.no_file_name(shown=not self.filename) + self.Error.no_file_name(shown=not self.filename and self.auto_save) self.Error.unsupported_sparse( shown=self.data is not None and self.data.is_sparse() From ae1b69bfeb85362bb3c0c2d35ec8d378387bc7dd Mon Sep 17 00:00:00 2001 From: janezd Date: Thu, 21 Feb 2019 14:43:04 +0100 Subject: [PATCH 06/12] OWSave: Further experiments with save dialog --- Orange/widgets/data/owsave.py | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index 28a9b1238d1..34ae5f45df2 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -9,6 +9,10 @@ from Orange.widgets.utils.widgetpreview import WidgetPreview from Orange.widgets.widget import Input +class FileDialog(QFileDialog): + def changeEvent(self, e): + print(e) + super().selectFile(e) class OWSave(widget.OWWidget): name = "Save Data" @@ -20,7 +24,7 @@ class OWSave(widget.OWWidget): settings_version = 2 writers = [TabReader, CSVReader, PickleReader, ExcelReader] - filters = [f"{w.DESCRIPTION} (*{w.EXTENSIONS[0]})" for w in writers] + filters = [f"{w.DESCRIPTION} (*.*)" for w in writers] filt_ext = {filter: w.EXTENSIONS[0] for filter, w in zip(filters, writers)} userhome = os.path.expanduser(f"~{os.sep}") @@ -108,8 +112,22 @@ def save_file_as(self): data_name += self.filt_ext[self.filter] start_dir = os.path.join(self.last_dir or self.userhome, data_name) - filename, selected_filter = QFileDialog.getSaveFileName( - self, "Save data", start_dir, ";;".join(self.filters), self.filter) + dlg = FileDialog(None, "Set File", start_dir, ";;".join(self.filters)) + dlg.setLabelText(dlg.Accept, "Select") + dlg.setAcceptMode(dlg.AcceptSave) + dlg.setSupportedSchemes(["file"]) + dlg.selectNameFilter(self.filter) + dlg.setOption(QFileDialog.HideNameFilterDetails) + dlg.currentChanged.connect(print) + if dlg.exec() == dlg.Rejected: + return + + filename = dlg.selectedFiles()[0] + selected_filter = dlg.selectedNameFilter() + +# filename, selected_filter = QFileDialog.getSaveFileName( + # self, "Save data", start_dir, ";;".join(self.filters), self.filter, + # QFileDialog.HideNameFilterDetails) if not filename: return @@ -186,8 +204,9 @@ def send_report(self): @classmethod def migrate_settings(cls, settings, version=0): - if version < 2: - settings["filter"] = settings.pop("filetype") + settings.filter = next(iter(cls.filt_ext)) +# if version < 2: + # settings["filter"] = settings.pop("filetype") if __name__ == "__main__": # pragma: no cover From 98a7415b612fd6a014b64a119e21a2062b3fa74a Mon Sep 17 00:00:00 2001 From: janezd Date: Sun, 24 Feb 2019 15:29:04 +0100 Subject: [PATCH 07/12] OWSave: Separate dialogs for different platforms --- Orange/widgets/data/owsave.py | 312 +++++++++++-------- Orange/widgets/data/tests/test_owsave.py | 363 +++++++++++++++-------- 2 files changed, 434 insertions(+), 241 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index 34ae5f45df2..694e45a2bc7 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -1,6 +1,8 @@ import os.path +import sys +import re -from AnyQt.QtWidgets import QFileDialog, QGridLayout, QWidget +from AnyQt.QtWidgets import QFileDialog, QGridLayout, QMessageBox from Orange.data.table import Table from Orange.data.io import TabReader, CSVReader, PickleReader, ExcelReader @@ -9,10 +11,6 @@ from Orange.widgets.utils.widgetpreview import WidgetPreview from Orange.widgets.widget import Input -class FileDialog(QFileDialog): - def changeEvent(self, e): - print(e) - super().selectFile(e) class OWSave(widget.OWWidget): name = "Save Data" @@ -24,170 +22,122 @@ class OWSave(widget.OWWidget): settings_version = 2 writers = [TabReader, CSVReader, PickleReader, ExcelReader] - filters = [f"{w.DESCRIPTION} (*.*)" for w in writers] - filt_ext = {filter: w.EXTENSIONS[0] for filter, w in zip(filters, writers)} + filters = { + **{f"{w.DESCRIPTION} (*{w.EXTENSIONS[0]})": w + for w in writers}, + **{f"Compressed {w.DESCRIPTION} (*{w.EXTENSIONS[0]}.gz)": w + for w in writers if w.SUPPORT_COMPRESSED} + } userhome = os.path.expanduser(f"~{os.sep}") class Inputs: data = Input("Data", Table) class Error(widget.OWWidget.Error): - unsupported_sparse = widget.Msg("Use .pkl format for sparse data.") + unsupported_sparse = widget.Msg("Use Pickle format for sparse data.") no_file_name = widget.Msg("File name is not set.") general_error = widget.Msg("{}") class Warning(widget.OWWidget.Warning): - ignored_flag = widget.Msg("{} ignored for this format.") + type_annotation_ignored = widget.Msg( + "Type annotation setting is ignored for this format.") want_main_area = False resizing_enabled = False - compress: bool - add_type_annotations: bool - last_dir = Setting("") - filter = Setting(filters[0]) - compress = Setting(False) + filter = Setting(next(iter(filters))) + filename = Setting("", schema_only=True) add_type_annotations = Setting(True) auto_save = Setting(False) def __init__(self): super().__init__() self.data = None - self.filename = "" - self.writer = self.writers[0] grid = QGridLayout() - gui.widgetBox(self.controlArea, box=True, orientation=grid) - grid.setSpacing(8) - self.bt_save = gui.button(None, self, "Save", callback=self.save_file) - grid.addWidget(self.bt_save, 0, 0) - grid.addWidget( - gui.button(None, self, "Save as ...", callback=self.save_file_as), - 0, 1) - grid.addWidget( - gui.checkBox(None, self, "auto_save", - "Autosave when receiving new data", - callback=self._update_controls), - 1, 0, 1, 2) - grid.addWidget(QWidget(), 2, 0, 1, 2) - + gui.widgetBox(self.controlArea, orientation=grid) grid.addWidget( gui.checkBox( None, self, "add_type_annotations", - "Save with type annotations", callback=self._update_controls), - 3, 0, 1, 2) + "Save with type annotations", callback=self._update_messages), + 0, 0, 1, 2) + grid.setRowMinimumHeight(1, 8) grid.addWidget( gui.checkBox( - None, self, "compress", "Compress file (gzip)", - callback=self._update_controls), - 4, 0, 1, 2) + None, self, "auto_save", "Autosave when receiving new data", + callback=self._update_messages), + 2, 0, 1, 2) + grid.setRowMinimumHeight(3, 8) + self.bt_save = gui.button(None, self, "Save", callback=self.save_file) + grid.addWidget(self.bt_save, 4, 0) + grid.addWidget( + gui.button(None, self, "Save as ...", callback=self.save_file_as), + 4, 1) self.adjustSize() - self._update_controls() + self._update_messages() + + @property + def writer(self): + return self.filters[self.filter] @Inputs.data def dataset(self, data): self.Error.clear() self.data = data - - self._update_controls() - if self.data is None: - self.info.set_input_summary(self.info.NoInput) - else: - self.info.set_input_summary( - str(len(self.data)), - f"Data set {self.data.name or '(no name)'} " - f"with {len(self.data)} instances") - + self._update_status() + self._update_messages() if self.auto_save and self.filename: self.save_file() - def save_file_as(self): - if self.filename: - start_dir = self.filename - else: - data_name = getattr(self.data, 'name', '') - if data_name: - data_name += self.filt_ext[self.filter] - start_dir = os.path.join(self.last_dir or self.userhome, data_name) - - dlg = FileDialog(None, "Set File", start_dir, ";;".join(self.filters)) - dlg.setLabelText(dlg.Accept, "Select") - dlg.setAcceptMode(dlg.AcceptSave) - dlg.setSupportedSchemes(["file"]) - dlg.selectNameFilter(self.filter) - dlg.setOption(QFileDialog.HideNameFilterDetails) - dlg.currentChanged.connect(print) - if dlg.exec() == dlg.Rejected: - return - - filename = dlg.selectedFiles()[0] - selected_filter = dlg.selectedNameFilter() - -# filename, selected_filter = QFileDialog.getSaveFileName( - # self, "Save data", start_dir, ";;".join(self.filters), self.filter, - # QFileDialog.HideNameFilterDetails) - if not filename: - return - - self.filename = filename - self.last_dir = os.path.split(filename)[0] - self.filter = selected_filter - self.writer = self.writers[self.filters.index(self.filter)] - self._update_controls() - self.save_file() - def save_file(self): if not self.filename: self.save_file_as() return + self.Error.general_error.clear() - if not self._can_save(): + if self.data is None \ + or not self.filename \ + or (self.data.is_sparse() + and not self.writer.SUPPORT_SPARSE_DATA): return try: self.writer.write( - self._fullname(), self.data, self.add_type_annotations) + self.filename, self.data, self.add_type_annotations) except IOError as err_value: self.Error.general_error(str(err_value)) - def _fullname(self): - return self.filename \ - + ".gz" * self.writer.SUPPORT_COMPRESSED * self.compress - - def _update_controls(self): - if self.filename: - self.bt_save.setText( - f"Save as {os.path.split(self._fullname())[1]}") - else: - self.bt_save.setText("Save") - self.Error.no_file_name(shown=not self.filename and self.auto_save) + def save_file_as(self): + filename, selected_filter = self.get_save_filename() + if not filename: + return + self.filename = filename + self.filter = selected_filter + self.last_dir = os.path.split(self.filename)[0] + self.bt_save.setText(f"Save as {os.path.split(filename)[1]}") + self._update_messages() + self.save_file() + def _update_messages(self): + self.Error.no_file_name( + shown=not self.filename and self.auto_save) self.Error.unsupported_sparse( shown=self.data is not None and self.data.is_sparse() and self.filename and not self.writer.SUPPORT_SPARSE_DATA) + self.Warning.type_annotation_ignored( + shown=self.data is not None and self.filename + and self.add_type_annotations + and not self.writer.OPTIONAL_TYPE_ANNOTATIONS) - if self.data is None or not self.filename: - self.Warning.ignored_flag.clear() + def _update_status(self): + if self.data is None: + self.info.set_input_summary(self.info.NoInput) else: - no_compress = self.compress \ - and not self.writer.SUPPORT_COMPRESSED - no_anotation = self.add_type_annotations \ - and not self.writer.OPTIONAL_TYPE_ANNOTATIONS - ignored = [ - "", - "Compression flag is", - "Type annotation flag is", - "Compression and type annotation flags are" - ][no_compress + 2 * no_anotation] - self.Warning.ignored_flag(ignored, shown=bool(ignored)) - - def _can_save(self): - return not ( - self.data is None - or not self.filename - or self.data.is_sparse() and not self.writer.SUPPORT_SPARSE_DATA - ) + self.info.set_input_summary( + str(len(self.data)), + f"Data set {self.data.name or '(no name)'} " + f"with {len(self.data)} instances") def send_report(self): self.report_data_brief(self.data) @@ -196,7 +146,6 @@ def send_report(self): self.report_items(( ("File name", self.filename or "not set"), ("Format", writer.DESCRIPTION), - ("Compression", writer.SUPPORT_COMPRESSED and noyes[self.compress]), ("Type annotations", writer.OPTIONAL_TYPE_ANNOTATIONS and noyes[self.add_type_annotations]) @@ -204,9 +153,136 @@ def send_report(self): @classmethod def migrate_settings(cls, settings, version=0): - settings.filter = next(iter(cls.filt_ext)) -# if version < 2: - # settings["filter"] = settings.pop("filetype") + def migrate_to_version_2(): + # Set the default; change later if possible + settings.pop("compression", None) + settings["filter"] = next(iter(cls.filters)) + filetype = settings.pop("filetype", None) + if filetype is None: + return + + ext = cls._extension_from_filter(filetype) + if settings.pop("compress", False): + for afilter in cls.filters: + if ext + ".gz" in afilter: + settings["filter"] = afilter + return + # If not found, uncompressed may have been erroneously set + # for a writer that didn't support if (such as .xlsx), so + # fall through to uncompressed + for afilter in cls.filters: + if ext in afilter: + settings["filter"] = afilter + return + + if version < 2: + migrate_to_version_2() + + + + def _initial_start_dir(self): + if self.filename and os.path.exists(os.path.split(self.filename)[0]): + return self.filename + else: + data_name = getattr(self.data, 'name', '') + if data_name: + data_name += self.writer.EXTENSIONS[0] + return os.path.join(self.last_dir or self.userhome, data_name) + + @staticmethod + def _replace_extension(filename, extension): + if filename.endswith(extension): # it may contain dots before extension + return filename + last_fn = None + while last_fn != filename: + last_fn, filename = filename, os.path.splitext(filename)[0] + return filename + extension + + @staticmethod + def _extension_from_filter(selected_filter): + return re.search(r".*\(\*?(\..*)\)$", selected_filter).group(1) + + # As of Qt 5.9, QFileDialog.setDefaultSuffix does not support double + # suffixes, not even in non-native dialogs. We handle each OS separately. + if sys.platform == "darwin": + # On macOS, is double suffixes are passed to the dialog, they are + # appended multiple times even if already present (QTBUG-44227). + # The only known workaround with native dialog is to use suffix *.*. + # We add the correct suffix after closing the dialog and only then check + # if the file exists and ask whether to override. + # It is a bit confusing that the user does not see the final name in the + # dialog, but I see no better solution. + def get_save_filename(self): # pragma: no cover + def no_suffix(filt): + return filt.split("(")[0] + "(*.*)" + + mac_filters = {no_suffix(f): f for f in self.filters} + filename = self._initial_start_dir() + while True: + dlg = QFileDialog( + None, "Save File", filename, ";;".join(mac_filters)) + dlg.setAcceptMode(dlg.AcceptSave) + dlg.selectNameFilter(no_suffix(self.filter)) + dlg.setOption(QFileDialog.HideNameFilterDetails) + dlg.setOption(QFileDialog.DontConfirmOverwrite) + if dlg.exec() == QFileDialog.Rejected: + return "", "" + filename = dlg.selectedFiles()[0] + selected_filter = mac_filters[dlg.selectedNameFilter()] + filename = self._replace_extension( + filename, self._extension_from_filter(selected_filter)) + if not os.path.exists(filename) or QMessageBox.question( + self, "Overwrite file?", + f"File {os.path.split(filename)[1]} already exists.\n" + "Overwrite?") == QMessageBox.Yes: + return filename, selected_filter + + elif sys.platform == "win32": + # TODO: This is not tested!!! + # Windows native dialog may work correctly; if not, we may do the same + # as for macOS? + def get_save_filename(self): # pragma: no cover + return QFileDialog.getSaveFileName( + self, "Save File", self._initial_start_dir(), + ";;".join(self.filters), self.filter) + + else: # Linux and any unknown platforms + # Qt does not use a native dialog on Linux, so we can connect to + # filterSelected and to overload selectFile to change the extension + # while the dialog is open. + # For unknown platforms (which?), we also use the non-native dialog to + # be sure we know what happens. + class SaveFileDialog(QFileDialog): + # pylint: disable=protected-access + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.setAcceptMode(QFileDialog.AcceptSave) + self.setOption(QFileDialog.DontUseNativeDialog) + self.filterSelected.connect(self.updateDefaultExtension) + + def selectNameFilter(self, selected_filter): + super().selectNameFilter(selected_filter) + self.updateDefaultExtension(selected_filter) + + def updateDefaultExtension(self, selected_filter): + self.suffix = OWSave._extension_from_filter(selected_filter) + files = self.selectedFiles() + if files and not os.path.isdir(files[0]): + self.selectFile(files[0].split(".")[0]) + + def selectFile(self, filename): + filename = OWSave._replace_extension(filename, self.suffix) + super().selectFile(filename) + + def get_save_filename(self): + dlg = self.SaveFileDialog( + None, "Save File", self._initial_start_dir(), + ";;".join(self.filters)) + dlg.selectNameFilter(self.filter) + if dlg.exec() == QFileDialog.Rejected: + return "", "" + else: + return dlg.selectedFiles()[0], dlg.selectedNameFilter() if __name__ == "__main__": # pragma: no cover diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index 9e1ce5fdb83..feee1df0cf5 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -2,20 +2,18 @@ import unittest from unittest.mock import patch, Mock import os +import sys import scipy.sparse as sp +from AnyQt.QtWidgets import QFileDialog from Orange.data import Table -from Orange.data.io import CSVReader, TabReader, PickleReader, ExcelReader +from Orange.data.io import TabReader, PickleReader, ExcelReader from Orange.tests import named_file from Orange.widgets.tests.base import WidgetTest from Orange.widgets.data.owsave import OWSave -FILE_TYPES = [("{} (*{})".format(w.DESCRIPTION, w.EXTENSIONS[0]), w) - for w in (TabReader, CSVReader, PickleReader, ExcelReader)] - - # Yay, MS Windows! # This is not the proper general way to do it, but it's simplest and sufficient # Short name is suitable for the function's purpose @@ -25,11 +23,19 @@ def _w(s): # pylint: disable=invalid-name class TestOWSave(WidgetTest): def setUp(self): - self.widget = self.create_widget(OWSave) # type: OWSave + class OWSaveMockWriter(OWSave): + writer = Mock() + writer.EXTENSIONS = [".csv"] + writer.SUPPORT_COMPRESSED = True + writer.SUPPORT_SPARSE_DATA = False + writer.OPTIONAL_TYPE_ANNOTATIONS = False + + self.widget = self.create_widget(OWSaveMockWriter) # type: OWSave self.iris = Table("iris") def test_dataset(self): widget = self.widget + widget.auto_save = True insum = widget.info.set_input_summary = Mock() savefile = widget.save_file = Mock() @@ -54,73 +60,130 @@ def test_dataset(self): self.send_signal(datasig, None) insum.assert_called_with(widget.info.NoInput) - @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") - def test_save_file_as_start_dir(self, filedialog): + @unittest.skipUnless(sys.platform == "linux", "Test Qt's non-native dialog") + def test_get_save_filename_non_native(self): + widget = self.widget + widget._initial_start_dir = lambda: "baz" + widget.filters = dict.fromkeys("abc") + widget.filter = "b" + dlg = widget.SaveFileDialog = Mock() # pylint: disable=invalid-name + instance = dlg.return_value + instance.selectedFiles.return_value = ["foo"] + instance.selectedNameFilter.return_value = "bar" + self.assertEqual(widget.get_save_filename(), ("foo", "bar")) + self.assertEqual(dlg.call_args[0][2], "baz") + self.assertEqual(dlg.call_args[0][3], "a;;b;;c") + instance.selectNameFilter.assert_called_with("b") + + instance.exec.return_value = QFileDialog.Rejected + self.assertEqual(widget.get_save_filename(), ("", "")) + + @unittest.skipUnless(sys.platform == "linux", "Test Qt's non-native dialog") + def test_save_file_dialog_enforces_extension(self): + dialog = OWSave.SaveFileDialog( + None, "Save File", "high.txt", + "Bar files (*.bar);;Low files (*.low)") + + dialog.selectNameFilter("Low files (*.low)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + + dialog.selectFile("high.txt") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + + dialog.selectNameFilter("Bar files (*.bar)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar")) + + dialog.selectFile("middle.txt") + self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.bar")) + + dialog.filterSelected.emit("Low files (*.low)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.low")) + + dialog.selectFile("high.txt") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + + def test_initial_start_dir(self): widget = self.widget widget.filename = _w("/usr/foo/bar.csv") - widget.filter = FILE_TYPES[1][0] - - filedialog.return_value = "", FILE_TYPES[0][0] + self.assertEqual(widget._initial_start_dir(), + _w(os.path.expanduser("~/"))) - widget.save_file_as() - self.assertEqual(filedialog.call_args[0][2], widget.filename) + with patch("os.path.exists", return_value=True): + widget.filename = _w("/usr/foo/bar.csv") + self.assertEqual(widget._initial_start_dir(), widget.filename) - widget.filename = "" - widget.last_dir = _w("/usr/bar") - widget.save_file_as() - self.assertEqual(filedialog.call_args[0][2], _w("/usr/bar/")) + widget.filename = "" + widget.last_dir = _w("/usr/bar") + self.assertEqual(widget._initial_start_dir(), _w("/usr/bar/")) - self.send_signal(widget.Inputs.data, self.iris) - widget.last_dir = _w("/usr/bar") - widget.save_file_as() - self.assertEqual(filedialog.call_args[0][2], _w("/usr/bar/iris.csv")) + widget.last_dir = _w("/usr/bar") + self.send_signal(widget.Inputs.data, self.iris) + self.assertEqual(widget._initial_start_dir(), + _w("/usr/bar/iris.csv")) - widget.last_dir = "" - widget.save_file_as() - self.assertEqual(filedialog.call_args[0][2], - os.path.expanduser(_w("~/iris.csv"))) + widget.last_dir = "" + self.assertEqual(widget._initial_start_dir(), + os.path.expanduser(_w("~/iris.csv"))) @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") - def test_save_file_as_name(self, filedialog): + def test_save_file_sets_name(self, filedialog): widget = self.widget + filters = iter(widget.filters) + filter1 = next(filters) + filter2 = next(filters) + widget.filename = _w("/usr/foo/bar.csv") widget.last_dir = _w("/usr/foo/") - widget.writer = widget.writers[1] - widget.filter = FILE_TYPES[1][0] + widget.filter = filter1 - widget._update_controls = Mock() + widget._update_messages = Mock() widget.save_file = Mock() - filedialog.return_value = "", FILE_TYPES[0][0] + widget.get_save_filename = Mock(return_value=("", filter2)) widget.save_file_as() self.assertEqual(widget.filename, _w("/usr/foo/bar.csv")) self.assertEqual(widget.last_dir, _w("/usr/foo/")) - self.assertEqual(widget.filter, FILE_TYPES[1][0]) - self.assertIs(widget.writer, widget.writers[1]) - widget._update_controls.assert_not_called() + self.assertEqual(widget.filter, filter1) + widget._update_messages.assert_not_called() widget.save_file.assert_not_called() - filedialog.return_value = _w("/bar/baz.csv"), FILE_TYPES[0][0] + widget.get_save_filename = \ + Mock(return_value=(_w("/bar/bar.csv"), filter2)) + widget.save_file_as() + self.assertEqual(widget.filename, _w("/bar/bar.csv")) + self.assertEqual(widget.last_dir, _w("/bar")) + self.assertEqual(widget.filter, filter2) + self.assertIn("bar.csv", widget.bt_save.text()) + widget._update_messages.assert_called() + widget.save_file.assert_called() + + widget.get_save_filename = Mock(return_value=("", filter2)) widget.save_file_as() - self.assertEqual(widget.filename, _w("/bar/baz.csv")) + self.assertEqual(widget.filename, _w("/bar/bar.csv")) self.assertEqual(widget.last_dir, _w("/bar")) - self.assertEqual(widget.filter, FILE_TYPES[0][0]) - self.assertIs(widget.writer, widget.writers[0]) - widget._update_controls.assert_called() + self.assertEqual(widget.filter, filter2) + self.assertIn("bar.csv", widget.bt_save.text()) + widget._update_messages.assert_called() widget.save_file.assert_called() - def set_mock_writer(self): + def test_save_file_calls_save_as(self): widget = self.widget - writer = widget.writer = Mock() - writer.write = Mock() - writer.SUPPORT_COMPRESSED = True - writer.SUPPORT_SPARSE_DATA = False - writer.OPTIONAL_TYPE_ANNOTATIONS = False + widget.save_file_as = Mock() + + self.send_signal(widget.Inputs.data, self.iris) - def test_save_file_check_can_save(self): + widget.filename = "" + widget.save_file() + widget.save_file_as.assert_called() + widget.save_file_as.reset_mock() + + widget.filename = "bar.csv" + widget.save_file() + widget.save_file_as.assert_not_called() + + def test_save_file_checks_can_save(self): widget = self.widget - widget.save_file_as = Mock(return_value=("", 0)) - self.set_mock_writer() + widget.get_save_filename = Mock(return_value=("", 0)) widget.save_file() widget.writer.write.assert_not_called() @@ -153,7 +216,6 @@ def test_save_file_write_errors(self): widget.auto_save = True widget.filename = _w("bar/foo") - self.set_mock_writer() widget.writer.write.side_effect = IOError self.send_signal(datasig, self.iris) @@ -178,32 +240,28 @@ def test_save_file_write(self): widget = self.widget datasig = widget.Inputs.data - self.set_mock_writer() widget.auto_save = True widget.filename = _w("bar/foo.csv") - widget.compress = False widget.add_type_annotations = True self.send_signal(datasig, self.iris) widget.writer.write.assert_called_with( _w("bar/foo.csv"), self.iris, True) - widget.compress = True - self.send_signal(datasig, self.iris) - widget.writer.write.assert_called_with( - _w("bar/foo.csv.gz"), self.iris, True) - def test_file_name_label(self): widget = self.widget widget.filename = "" - widget._update_controls() + widget._update_messages() + self.assertFalse(widget.Error.no_file_name.is_shown()) + + widget.auto_save = True + widget._update_messages() self.assertTrue(widget.Error.no_file_name.is_shown()) widget.filename = _w("/foo/bar/baz.csv") - widget._update_controls() + widget._update_messages() self.assertFalse(widget.Error.no_file_name.is_shown()) - self.assertIn("baz.csv", widget.bt_save.text()) def test_sparse_error(self): widget = self.widget @@ -213,92 +271,131 @@ def test_sparse_error(self): widget.filename = "foo.xlsx" widget.data = self.iris - widget._update_controls() + widget._update_messages() self.assertFalse(err.is_shown()) widget.data.X = sp.csr_matrix(widget.data.X) - widget._update_controls() + widget._update_messages() self.assertTrue(err.is_shown()) widget.writer = PickleReader - widget._update_controls() + widget._update_messages() self.assertFalse(err.is_shown()) widget.writer = ExcelReader - widget._update_controls() + widget._update_messages() self.assertTrue(err.is_shown()) widget.data = None - widget._update_controls() + widget._update_messages() self.assertFalse(err.is_shown()) - def test_ignored_flags_warnings(self): + def test_ignored_annotation_warning(self): widget = self.widget widget.writer = ExcelReader - widget.compress = True widget.add_type_annotations = True - widget._update_controls() - self.assertFalse(widget.Warning.ignored_flag.is_shown()) + widget._update_messages() + self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) widget.filename = "test.xlsx" - widget._update_controls() - self.assertFalse(widget.Warning.ignored_flag.is_shown()) + widget._update_messages() + self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) self.send_signal(widget.Inputs.data, self.iris) - widget._update_controls() - self.assertTrue(widget.Warning.ignored_flag.is_shown()) - - widget.compress = False - widget._update_controls() - self.assertTrue(widget.Warning.ignored_flag.is_shown()) + widget._update_messages() + self.assertTrue(widget.Warning.type_annotation_ignored.is_shown()) widget.add_type_annotations = False - widget._update_controls() - self.assertFalse(widget.Warning.ignored_flag.is_shown()) + widget._update_messages() + self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) - widget.writer = PickleReader - widget.filename = "test.pkl" widget.add_type_annotations = True - widget.compress = False - widget._update_controls() - self.assertTrue(widget.Warning.ignored_flag.is_shown()) + widget._update_messages() + self.assertTrue(widget.Warning.type_annotation_ignored.is_shown()) - widget.add_type_annotations = False - widget.compress = True - widget._update_controls() - self.assertFalse(widget.Warning.ignored_flag.is_shown()) + widget.writer = TabReader + widget._update_messages() + self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) def test_send_report(self): widget = self.widget widget.report_items = Mock() - for _, writer in FILE_TYPES: + for writer in widget.filters.values(): widget.writer = writer - for widget.compress in (False, True): - for widget.add_type_annotations in (False, True): - widget.filename = f"foo.{writer.EXTENSIONS[0]}" - widget.send_report() - items = dict(widget.report_items.call_args[0][0]) - msg = f"for {widget.writer}, " \ - f"annotations={widget.add_type_annotations}, " \ - f"compress={widget.compress}" - self.assertEqual(items["File name"], - f"foo.{writer.EXTENSIONS[0]}", msg=msg) - if writer.SUPPORT_COMPRESSED: - self.assertEqual( - items["Compression"], - ["No", "Yes"][widget.compress], - msg=msg) - else: - self.assertFalse(items["Compression"], msg=msg) - if writer.OPTIONAL_TYPE_ANNOTATIONS: - self.assertEqual( - items["Type annotations"], - ["No", "Yes"][widget.add_type_annotations], - msg=msg) - else: - self.assertFalse(items["Type annotations"], msg=msg) + for widget.add_type_annotations in (False, True): + widget.filename = f"foo.{writer.EXTENSIONS[0]}" + widget.send_report() + items = dict(widget.report_items.call_args[0][0]) + msg = f"for {writer}, annotations={widget.add_type_annotations}" + self.assertEqual(items["File name"], widget.filename, msg=msg) + if writer.OPTIONAL_TYPE_ANNOTATIONS: + self.assertEqual( + items["Type annotations"], + ["No", "Yes"][widget.add_type_annotations], msg=msg) + else: + self.assertFalse(items["Type annotations"], msg=msg) + + def test_migration_to_version_2(self): + const_settings = { + 'add_type_annotations': True, 'auto_save': False, + 'controlAreaVisible': True, 'last_dir': '/home/joe/Desktop', + '__version__': 1} + + # No compression, Tab-separated values + settings = {**const_settings, + 'compress': False, 'compression': 'gzip (.gz)', + 'filetype': 'Tab-separated values (.tab)'} + OWSave.migrate_settings(settings) + self.assertEqual( + settings, + {**const_settings, + "filter": "Tab-separated values (*.tab)"}) + + # Compression; ignore compression format (.xz is no longer supported) + settings = {**const_settings, + 'compress': True, 'compression': 'lzma (.xz)', + 'filetype': 'Tab-separated values (.tab)'} + OWSave.migrate_settings(settings) + self.assertEqual( + settings, + {**const_settings, + "filter": "Compressed Tab-separated values (*.tab.gz)"}) + + # No compression, Excel + settings = {**const_settings, + 'compress': False, 'compression': 'lzma (.xz)', + 'filetype': 'Microsoft Excel spreadsheet (.xlsx)'} + OWSave.migrate_settings(settings) + self.assertEqual( + settings, + {**const_settings, + "filter": "Microsoft Excel spreadsheet (*.xlsx)"}) + + # Excel with compression - compression must be ignored + settings = {**const_settings, + 'compress': True, 'compression': 'lzma (.xz)', + 'filetype': 'Microsoft Excel spreadsheet (.xlsx)'} + OWSave.migrate_settings(settings) + self.assertEqual( + settings, + {**const_settings, + "filter": "Microsoft Excel spreadsheet (*.xlsx)"}) + + # Missing filetype (is this possible?) + settings = {**const_settings, + 'compress': True, 'compression': 'lzma (.xz)'} + OWSave.migrate_settings(settings) + self.assertTrue(settings["filter"] in OWSave.filters) + + # Unsupported file format (is this possible?) + settings = {**const_settings, + 'compress': True, 'compression': 'lzma (.xz)', + 'filetype': 'Bar file (.bar)'} + OWSave.migrate_settings(settings) + self.assertTrue(settings["filter"] in OWSave.filters) + class TestFunctionalOWSave(WidgetTest): @@ -306,19 +403,19 @@ def setUp(self): self.widget = self.create_widget(OWSave) # type: OWSave self.iris = Table("iris") - @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") - def test_save_uncompressed(self, filedialog): + def test_save_uncompressed(self): widget = self.widget widget.auto_save = False spiris = Table("iris") spiris.X = sp.csr_matrix(spiris.X) - for selected_filter, writer in FILE_TYPES: + for selected_filter, writer in widget.filters.items(): widget.write = writer ext = writer.EXTENSIONS[0] with named_file("", suffix=ext) as filename: - filedialog.return_value = filename, selected_filter + widget.get_save_filename = Mock( + return_value=(filename, selected_filter)) self.send_signal(widget.Inputs.data, self.iris) widget.save_file_as() @@ -329,14 +426,34 @@ def test_save_uncompressed(self, filedialog): widget.save_file() self.assertEqual(len(Table(filename)), 150) - if writer.SUPPORT_COMPRESSED: - with named_file("", suffix=ext + ".gz") as filename: - filedialog.return_value = filename[:-3], selected_filter - widget.compress = True - self.send_signal(widget.Inputs.data, self.iris) - widget.save_file_as() - self.assertEqual(len(Table(filename)), 150) - widget.compress = False + +class TestOWSaveUtils(unittest.TestCase): + def test_replace_extension(self): + replace = OWSave._replace_extension + fname = "/bing.bada.boom/foo.bar.baz" + self.assertEqual(replace(fname, ".baz"), fname) + self.assertEqual(replace(fname, ".bar.baz"), fname) + self.assertEqual(replace(fname, ".txt"), "/bing.bada.boom/foo.txt") + + fname = "foo.bar.baz" + self.assertEqual(replace(fname, ".baz"), fname) + self.assertEqual(replace(fname, ".bar.baz"), fname) + self.assertEqual(replace(fname, ".txt"), "foo.txt") + self.assertEqual(replace(fname, ".bar.txt"), "foo.bar.txt") + + fname = "/bing.bada.boom/foo" + self.assertEqual(replace(fname, ".baz"), fname + ".baz") + self.assertEqual(replace(fname, ".bar.baz"), fname + ".bar.baz") + + def test_extension_from_filter(self): + self.assertEqual( + OWSave._extension_from_filter("Description (*.ext)"), ".ext") + self.assertEqual( + OWSave._extension_from_filter("Description (*.foo.ba)"), ".foo.ba") + self.assertEqual( + OWSave._extension_from_filter("Description (.ext)"), ".ext") + self.assertEqual( + OWSave._extension_from_filter("Description (.foo.bar)"), ".foo.bar") if __name__ == "__main__": From 2ca2558b943bac5b09d282a6a876065e74fe425c Mon Sep 17 00:00:00 2001 From: janezd Date: Thu, 28 Feb 2019 17:17:00 +0100 Subject: [PATCH 08/12] OWSave: Remove warnings about ignored annotation check box --- Orange/widgets/data/owsave.py | 16 +++++--------- Orange/widgets/data/tests/test_owsave.py | 28 ------------------------ 2 files changed, 5 insertions(+), 39 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index 694e45a2bc7..fd5212b7ef1 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -38,10 +38,6 @@ class Error(widget.OWWidget.Error): no_file_name = widget.Msg("File name is not set.") general_error = widget.Msg("{}") - class Warning(widget.OWWidget.Warning): - type_annotation_ignored = widget.Msg( - "Type annotation setting is ignored for this format.") - want_main_area = False resizing_enabled = False @@ -60,7 +56,11 @@ def __init__(self): grid.addWidget( gui.checkBox( None, self, "add_type_annotations", - "Save with type annotations", callback=self._update_messages), + "Add type annotations to header", + tooltip= + "Some formats (Tab-delimited, Comma-separated) can include \n" + "additional information about variables types in header rows.", + callback=self._update_messages), 0, 0, 1, 2) grid.setRowMinimumHeight(1, 8) grid.addWidget( @@ -125,10 +125,6 @@ def _update_messages(self): self.Error.unsupported_sparse( shown=self.data is not None and self.data.is_sparse() and self.filename and not self.writer.SUPPORT_SPARSE_DATA) - self.Warning.type_annotation_ignored( - shown=self.data is not None and self.filename - and self.add_type_annotations - and not self.writer.OPTIONAL_TYPE_ANNOTATIONS) def _update_status(self): if self.data is None: @@ -178,8 +174,6 @@ def migrate_to_version_2(): if version < 2: migrate_to_version_2() - - def _initial_start_dir(self): if self.filename and os.path.exists(os.path.split(self.filename)[0]): return self.filename diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index feee1df0cf5..be33324bce9 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -290,34 +290,6 @@ def test_sparse_error(self): widget._update_messages() self.assertFalse(err.is_shown()) - def test_ignored_annotation_warning(self): - widget = self.widget - - widget.writer = ExcelReader - widget.add_type_annotations = True - widget._update_messages() - self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) - - widget.filename = "test.xlsx" - widget._update_messages() - self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) - - self.send_signal(widget.Inputs.data, self.iris) - widget._update_messages() - self.assertTrue(widget.Warning.type_annotation_ignored.is_shown()) - - widget.add_type_annotations = False - widget._update_messages() - self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) - - widget.add_type_annotations = True - widget._update_messages() - self.assertTrue(widget.Warning.type_annotation_ignored.is_shown()) - - widget.writer = TabReader - widget._update_messages() - self.assertFalse(widget.Warning.type_annotation_ignored.is_shown()) - def test_send_report(self): widget = self.widget From c56fafab522f12165258439d7bd133a6c5d4cc12 Mon Sep 17 00:00:00 2001 From: janezd Date: Thu, 28 Feb 2019 17:18:00 +0100 Subject: [PATCH 09/12] OWSave: Strip only known extensions when enforcing default extensions --- Orange/widgets/data/owsave.py | 10 +++++----- Orange/widgets/data/tests/test_owsave.py | 25 +++++++++++++----------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index fd5212b7ef1..37ba1b3dc87 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -185,11 +185,11 @@ def _initial_start_dir(self): @staticmethod def _replace_extension(filename, extension): - if filename.endswith(extension): # it may contain dots before extension - return filename - last_fn = None - while last_fn != filename: - last_fn, filename = filename, os.path.splitext(filename)[0] + known_extensions = map(OWSave._extension_from_filter, OWSave.filters) + for known_ext in sorted(known_extensions, key=len, reverse=True): + if filename.endswith(known_ext): + filename = filename[:-len(known_ext)] + break return filename + extension @staticmethod diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index be33324bce9..9b6b317661c 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -402,20 +402,23 @@ def test_save_uncompressed(self): class TestOWSaveUtils(unittest.TestCase): def test_replace_extension(self): replace = OWSave._replace_extension - fname = "/bing.bada.boom/foo.bar.baz" - self.assertEqual(replace(fname, ".baz"), fname) - self.assertEqual(replace(fname, ".bar.baz"), fname) - self.assertEqual(replace(fname, ".txt"), "/bing.bada.boom/foo.txt") + fname = "/bing.bada.boom/foo.1942.tab" + self.assertEqual( + replace(fname, ".tab"), "/bing.bada.boom/foo.1942.tab") + self.assertEqual( + replace(fname, ".tab.gz"), "/bing.bada.boom/foo.1942.tab.gz") + self.assertEqual( + replace(fname, ".xlsx"), "/bing.bada.boom/foo.1942.xlsx") - fname = "foo.bar.baz" - self.assertEqual(replace(fname, ".baz"), fname) - self.assertEqual(replace(fname, ".bar.baz"), fname) - self.assertEqual(replace(fname, ".txt"), "foo.txt") - self.assertEqual(replace(fname, ".bar.txt"), "foo.bar.txt") + fname = "foo.tab.gz" + self.assertEqual(replace(fname, ".tab"), "foo.tab") + self.assertEqual(replace(fname, ".tab.gz"), "foo.tab.gz") + self.assertEqual(replace(fname, ".csv"), "foo.csv") + self.assertEqual(replace(fname, ".csv.gz"), "foo.csv.gz") fname = "/bing.bada.boom/foo" - self.assertEqual(replace(fname, ".baz"), fname + ".baz") - self.assertEqual(replace(fname, ".bar.baz"), fname + ".bar.baz") + self.assertEqual(replace(fname, ".tab"), fname + ".tab") + self.assertEqual(replace(fname, ".tab.gz"), fname + ".tab.gz") def test_extension_from_filter(self): self.assertEqual( From 546b3989ce6ea5536bccf76b8e41ba1f98def7f7 Mon Sep 17 00:00:00 2001 From: janezd Date: Thu, 28 Feb 2019 17:18:24 +0100 Subject: [PATCH 10/12] OWSave: Add tests for macOS --- Orange/widgets/data/tests/test_owsave.py | 101 ++++++++++++++++++++++- 1 file changed, 97 insertions(+), 4 deletions(-) diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index 9b6b317661c..8ab20b5b646 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -3,6 +3,7 @@ from unittest.mock import patch, Mock import os import sys +import re import scipy.sparse as sp from AnyQt.QtWidgets import QFileDialog @@ -60,8 +61,8 @@ def test_dataset(self): self.send_signal(datasig, None) insum.assert_called_with(widget.info.NoInput) - @unittest.skipUnless(sys.platform == "linux", "Test Qt's non-native dialog") - def test_get_save_filename_non_native(self): + @unittest.skipUnless(sys.platform == "linux", "Test for dialog on Linux") + def test_get_save_filename_linux(self): widget = self.widget widget._initial_start_dir = lambda: "baz" widget.filters = dict.fromkeys("abc") @@ -78,8 +79,8 @@ def test_get_save_filename_non_native(self): instance.exec.return_value = QFileDialog.Rejected self.assertEqual(widget.get_save_filename(), ("", "")) - @unittest.skipUnless(sys.platform == "linux", "Test Qt's non-native dialog") - def test_save_file_dialog_enforces_extension(self): + @unittest.skipUnless(sys.platform == "linux", "Test for dialog on Linux") + def test_save_file_dialog_enforces_extension_linux(self): dialog = OWSave.SaveFileDialog( None, "Save File", "high.txt", "Bar files (*.bar);;Low files (*.low)") @@ -102,6 +103,98 @@ def test_save_file_dialog_enforces_extension(self): dialog.selectFile("high.txt") self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + @unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_get_save_filename_darwin(self, dlg): + widget = self.widget + widget._initial_start_dir = lambda: "baz" + widget.filters = dict.fromkeys(("a (*.a)", "b (*.b)", "c (*.c)")) + widget.filter = "b (*.b)" + instance = dlg.return_value + instance.exec.return_value = dlg.Accepted = QFileDialog.Accepted + instance.selectedFiles.return_value = ["foo"] + instance.selectedNameFilter.return_value = "a (*.*)" + self.assertEqual(widget.get_save_filename(), ("foo.a", "a (*.a)")) + self.assertEqual(dlg.call_args[0][2], "baz") + self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*);;c (*.*)") + instance.selectNameFilter.assert_called_with("b (*.*)") + + instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected + self.assertEqual(widget.get_save_filename(), ("", "")) + + @staticmethod + def _mac_filter(filt): + return re.sub(r"\(\*\..*\)", "(*.*)", filt) + + @unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_save_file_dialog_enforces_extension_darwin(self, dlg): + widget = self.widget + for filter1 in widget.filters: + if OWSave._extension_from_filter(filter1) == ".tab": + break + for filter2 in widget.filters: + if OWSave._extension_from_filter(filter2) == ".csv.gz": + break + + widget.filter = filter1 + instance = dlg.return_value + instance.exec.return_value = QFileDialog.Accepted + + instance.selectedNameFilter.return_value = self._mac_filter(filter1) + instance.selectedFiles.return_value = ["foo"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.pkl"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.tab.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.csv.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.bar"] + self.assertEqual(widget.get_save_filename()[0], "foo.bar.tab") + + instance.selectedNameFilter.return_value = self._mac_filter(filter2) + instance.selectedFiles.return_value = ["foo"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.pkl"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.tab.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.csv.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.bar"] + self.assertEqual(widget.get_save_filename()[0], "foo.bar.csv.gz") + + @unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") + @patch("Orange.widgets.data.owsave.QFileDialog") + @patch("os.path.exists", new=lambda x: x == "old.tab") + @patch("Orange.widgets.data.owsave.QMessageBox") + def test_save_file_dialog_asks_for_overwrite_darwin(self, msgbox, dlg): + def selected_files(): + nonlocal attempts + attempts += 1 + return [["old.tab", "new.tab"][attempts]] + + widget = self.widget + widget._initial_start_dir = lambda: "baz" + for filter1 in widget.filters: + if OWSave._extension_from_filter(filter1) == ".tab": + break + + widget.filter = filter1 + instance = dlg.return_value + instance.exec.return_value = QFileDialog.Accepted + instance.selectedFiles = selected_files + instance.selectedNameFilter.return_value = self._mac_filter(filter1) + + attempts = -1 + msgbox.question.return_value = msgbox.Yes = 1 + self.assertEqual(widget.get_save_filename()[0], "old.tab") + + attempts = -1 + msgbox.question.return_value = msgbox.No = 0 + self.assertEqual(widget.get_save_filename()[0], "new.tab") + def test_initial_start_dir(self): widget = self.widget widget.filename = _w("/usr/foo/bar.csv") From 02d49dfa957271721cea1f1355e06f9229f6111e Mon Sep 17 00:00:00 2001 From: janezd Date: Thu, 28 Feb 2019 21:06:58 +0100 Subject: [PATCH 11/12] OWSave: Allow only formats that support sparse when data is sparse; add and refactor tests --- Orange/widgets/data/owsave.py | 29 +- Orange/widgets/data/tests/test_owsave.py | 343 ++++++++++++++--------- 2 files changed, 229 insertions(+), 143 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index 37ba1b3dc87..b79805d2ca6 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -196,6 +196,23 @@ def _replace_extension(filename, extension): def _extension_from_filter(selected_filter): return re.search(r".*\(\*?(\..*)\)$", selected_filter).group(1) + def _valid_filters(self): + if self.data is None or not self.data.is_sparse(): + return self.filters + else: + return {filt: writer for filt, writer in self.filters.items() + if writer.SUPPORT_SPARSE_DATA} + + def _default_valid_filter(self): + if self.data is None or not self.data.is_sparse() \ + or self.filters[self.filter].SUPPORT_SPARSE_DATA: + return self.filter + for filt, writer in self.filters.items(): + if writer.SUPPORT_SPARSE_DATA: + return filt + # This shouldn't happen and it will trigger an error in tests + return None # pragma: no cover + # As of Qt 5.9, QFileDialog.setDefaultSuffix does not support double # suffixes, not even in non-native dialogs. We handle each OS separately. if sys.platform == "darwin": @@ -210,13 +227,13 @@ def get_save_filename(self): # pragma: no cover def no_suffix(filt): return filt.split("(")[0] + "(*.*)" - mac_filters = {no_suffix(f): f for f in self.filters} + mac_filters = {no_suffix(f): f for f in self._valid_filters()} filename = self._initial_start_dir() while True: dlg = QFileDialog( None, "Save File", filename, ";;".join(mac_filters)) dlg.setAcceptMode(dlg.AcceptSave) - dlg.selectNameFilter(no_suffix(self.filter)) + dlg.selectNameFilter(no_suffix(self._default_valid_filter())) dlg.setOption(QFileDialog.HideNameFilterDetails) dlg.setOption(QFileDialog.DontConfirmOverwrite) if dlg.exec() == QFileDialog.Rejected: @@ -232,13 +249,13 @@ def no_suffix(filt): return filename, selected_filter elif sys.platform == "win32": - # TODO: This is not tested!!! + # TODO: Behaviour of getSaveFileName on Windows is not tested!!! # Windows native dialog may work correctly; if not, we may do the same # as for macOS? def get_save_filename(self): # pragma: no cover return QFileDialog.getSaveFileName( self, "Save File", self._initial_start_dir(), - ";;".join(self.filters), self.filter) + ";;".join(self._valid_filters()), self._default_valid_filter()) else: # Linux and any unknown platforms # Qt does not use a native dialog on Linux, so we can connect to @@ -271,8 +288,8 @@ def selectFile(self, filename): def get_save_filename(self): dlg = self.SaveFileDialog( None, "Save File", self._initial_start_dir(), - ";;".join(self.filters)) - dlg.selectNameFilter(self.filter) + ";;".join(self._valid_filters())) + dlg.selectNameFilter(self._default_valid_filter()) if dlg.exec() == QFileDialog.Rejected: return "", "" else: diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index 8ab20b5b646..031eca9fad2 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -1,4 +1,4 @@ -# pylint: disable=missing-docstring, protected-access +# pylint: disable=missing-docstring, protected-access, unsubscriptable-object import unittest from unittest.mock import patch, Mock import os @@ -22,7 +22,7 @@ def _w(s): # pylint: disable=invalid-name return s.replace("/", os.sep) -class TestOWSave(WidgetTest): +class TestOWSaveBase(WidgetTest): def setUp(self): class OWSaveMockWriter(OWSave): writer = Mock() @@ -34,6 +34,8 @@ class OWSaveMockWriter(OWSave): self.widget = self.create_widget(OWSaveMockWriter) # type: OWSave self.iris = Table("iris") + +class TestOWSave(TestOWSaveBase): def test_dataset(self): widget = self.widget widget.auto_save = True @@ -61,140 +63,6 @@ def test_dataset(self): self.send_signal(datasig, None) insum.assert_called_with(widget.info.NoInput) - @unittest.skipUnless(sys.platform == "linux", "Test for dialog on Linux") - def test_get_save_filename_linux(self): - widget = self.widget - widget._initial_start_dir = lambda: "baz" - widget.filters = dict.fromkeys("abc") - widget.filter = "b" - dlg = widget.SaveFileDialog = Mock() # pylint: disable=invalid-name - instance = dlg.return_value - instance.selectedFiles.return_value = ["foo"] - instance.selectedNameFilter.return_value = "bar" - self.assertEqual(widget.get_save_filename(), ("foo", "bar")) - self.assertEqual(dlg.call_args[0][2], "baz") - self.assertEqual(dlg.call_args[0][3], "a;;b;;c") - instance.selectNameFilter.assert_called_with("b") - - instance.exec.return_value = QFileDialog.Rejected - self.assertEqual(widget.get_save_filename(), ("", "")) - - @unittest.skipUnless(sys.platform == "linux", "Test for dialog on Linux") - def test_save_file_dialog_enforces_extension_linux(self): - dialog = OWSave.SaveFileDialog( - None, "Save File", "high.txt", - "Bar files (*.bar);;Low files (*.low)") - - dialog.selectNameFilter("Low files (*.low)") - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) - - dialog.selectFile("high.txt") - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) - - dialog.selectNameFilter("Bar files (*.bar)") - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar")) - - dialog.selectFile("middle.txt") - self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.bar")) - - dialog.filterSelected.emit("Low files (*.low)") - self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.low")) - - dialog.selectFile("high.txt") - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) - - @unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") - @patch("Orange.widgets.data.owsave.QFileDialog") - def test_get_save_filename_darwin(self, dlg): - widget = self.widget - widget._initial_start_dir = lambda: "baz" - widget.filters = dict.fromkeys(("a (*.a)", "b (*.b)", "c (*.c)")) - widget.filter = "b (*.b)" - instance = dlg.return_value - instance.exec.return_value = dlg.Accepted = QFileDialog.Accepted - instance.selectedFiles.return_value = ["foo"] - instance.selectedNameFilter.return_value = "a (*.*)" - self.assertEqual(widget.get_save_filename(), ("foo.a", "a (*.a)")) - self.assertEqual(dlg.call_args[0][2], "baz") - self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*);;c (*.*)") - instance.selectNameFilter.assert_called_with("b (*.*)") - - instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected - self.assertEqual(widget.get_save_filename(), ("", "")) - - @staticmethod - def _mac_filter(filt): - return re.sub(r"\(\*\..*\)", "(*.*)", filt) - - @unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") - @patch("Orange.widgets.data.owsave.QFileDialog") - def test_save_file_dialog_enforces_extension_darwin(self, dlg): - widget = self.widget - for filter1 in widget.filters: - if OWSave._extension_from_filter(filter1) == ".tab": - break - for filter2 in widget.filters: - if OWSave._extension_from_filter(filter2) == ".csv.gz": - break - - widget.filter = filter1 - instance = dlg.return_value - instance.exec.return_value = QFileDialog.Accepted - - instance.selectedNameFilter.return_value = self._mac_filter(filter1) - instance.selectedFiles.return_value = ["foo"] - self.assertEqual(widget.get_save_filename()[0], "foo.tab") - instance.selectedFiles.return_value = ["foo.pkl"] - self.assertEqual(widget.get_save_filename()[0], "foo.tab") - instance.selectedFiles.return_value = ["foo.tab.gz"] - self.assertEqual(widget.get_save_filename()[0], "foo.tab") - instance.selectedFiles.return_value = ["foo.csv.gz"] - self.assertEqual(widget.get_save_filename()[0], "foo.tab") - instance.selectedFiles.return_value = ["foo.bar"] - self.assertEqual(widget.get_save_filename()[0], "foo.bar.tab") - - instance.selectedNameFilter.return_value = self._mac_filter(filter2) - instance.selectedFiles.return_value = ["foo"] - self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") - instance.selectedFiles.return_value = ["foo.pkl"] - self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") - instance.selectedFiles.return_value = ["foo.tab.gz"] - self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") - instance.selectedFiles.return_value = ["foo.csv.gz"] - self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") - instance.selectedFiles.return_value = ["foo.bar"] - self.assertEqual(widget.get_save_filename()[0], "foo.bar.csv.gz") - - @unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") - @patch("Orange.widgets.data.owsave.QFileDialog") - @patch("os.path.exists", new=lambda x: x == "old.tab") - @patch("Orange.widgets.data.owsave.QMessageBox") - def test_save_file_dialog_asks_for_overwrite_darwin(self, msgbox, dlg): - def selected_files(): - nonlocal attempts - attempts += 1 - return [["old.tab", "new.tab"][attempts]] - - widget = self.widget - widget._initial_start_dir = lambda: "baz" - for filter1 in widget.filters: - if OWSave._extension_from_filter(filter1) == ".tab": - break - - widget.filter = filter1 - instance = dlg.return_value - instance.exec.return_value = QFileDialog.Accepted - instance.selectedFiles = selected_files - instance.selectedNameFilter.return_value = self._mac_filter(filter1) - - attempts = -1 - msgbox.question.return_value = msgbox.Yes = 1 - self.assertEqual(widget.get_save_filename()[0], "old.tab") - - attempts = -1 - msgbox.question.return_value = msgbox.No = 0 - self.assertEqual(widget.get_save_filename()[0], "new.tab") - def test_initial_start_dir(self): widget = self.widget widget.filename = _w("/usr/foo/bar.csv") @@ -383,6 +251,36 @@ def test_sparse_error(self): widget._update_messages() self.assertFalse(err.is_shown()) + def test_valid_filters_for_sparse(self): + widget = self.widget + + widget.data = None + self.assertEqual(widget.filters, widget._valid_filters()) + + widget.data = self.iris + self.assertEqual(widget.filters, widget._valid_filters()) + + widget.data.X = sp.csr_matrix(widget.data.X) + valid = widget._valid_filters() + self.assertNotEqual(widget.filters, {}) + self.assertTrue(all(v.SUPPORT_SPARSE_DATA for v in valid.values())) + + def test_valid_default_filter(self): + widget = self.widget + for widget.filter, writer in widget.filters.items(): + if not writer.SUPPORT_SPARSE_DATA: + break + + widget.data = None + self.assertIs(widget.filter, widget._default_valid_filter()) + + widget.data = self.iris + self.assertIs(widget.filter, widget._default_valid_filter()) + + widget.data.X = sp.csr_matrix(widget.data.X) + self.assertTrue( + widget.filters[widget._default_valid_filter()].SUPPORT_SPARSE_DATA) + def test_send_report(self): widget = self.widget @@ -462,7 +360,6 @@ def test_migration_to_version_2(self): self.assertTrue(settings["filter"] in OWSave.filters) - class TestFunctionalOWSave(WidgetTest): def setUp(self): self.widget = self.create_widget(OWSave) # type: OWSave @@ -492,6 +389,177 @@ def test_save_uncompressed(self): self.assertEqual(len(Table(filename)), 150) +@unittest.skipUnless(sys.platform == "linux", "Tests for dialog on Linux") +class TestOWSaveLinuxDialog(TestOWSaveBase): + def test_get_save_filename_linux(self): + widget = self.widget + widget._initial_start_dir = lambda: "baz" + widget.filters = dict.fromkeys("abc") + widget.filter = "b" + dlg = widget.SaveFileDialog = Mock() # pylint: disable=invalid-name + instance = dlg.return_value + instance.selectedFiles.return_value = ["foo"] + instance.selectedNameFilter.return_value = "bar" + self.assertEqual(widget.get_save_filename(), ("foo", "bar")) + self.assertEqual(dlg.call_args[0][2], "baz") + self.assertEqual(dlg.call_args[0][3], "a;;b;;c") + instance.selectNameFilter.assert_called_with("b") + + instance.exec.return_value = QFileDialog.Rejected + self.assertEqual(widget.get_save_filename(), ("", "")) + + def test_save_file_dialog_enforces_extension_linux(self): + dialog = OWSave.SaveFileDialog( + None, "Save File", "high.txt", + "Bar files (*.bar);;Low files (*.low)") + + dialog.selectNameFilter("Low files (*.low)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + + dialog.selectFile("high.txt") + print(dialog.selectedFiles()[0]) + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.txt.low")) + + dialog.selectNameFilter("Bar files (*.bar)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar")) + + dialog.selectFile("middle.pkl") + self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.bar")) + + dialog.filterSelected.emit("Low files (*.low)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.low")) + + dialog.selectFile("high.tab.gz") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + + def test_save_file_dialog_uses_valid_filters_linux(self): + widget = self.widget + widget._valid_filters = lambda: ["a (*.a)", "b (*.b)"] + widget._default_valid_filter = lambda: "a (*.a)" + dlg = widget.SaveFileDialog = Mock() # pylint: disable=invalid-name + instance = dlg.return_value + instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected + widget.get_save_filename() + self.assertEqual(dlg.call_args[0][3], "a (*.a);;b (*.b)") + instance.selectNameFilter.assert_called_with("a (*.a)") + + +@unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") +class TestOWSaveDarwinDialog(TestOWSaveBase): # pragma: no cover + @staticmethod + def _mac_filter(filt): + return re.sub(r"\(\*\..*\)", "(*.*)", filt) + + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_get_save_filename_darwin(self, dlg): + widget = self.widget + widget._initial_start_dir = lambda: "baz" + widget.filters = dict.fromkeys(("a (*.a)", "b (*.b)", "c (*.c)")) + widget.filter = "b (*.b)" + instance = dlg.return_value + instance.exec.return_value = dlg.Accepted = QFileDialog.Accepted + instance.selectedFiles.return_value = ["foo"] + instance.selectedNameFilter.return_value = "a (*.*)" + self.assertEqual(widget.get_save_filename(), ("foo.a", "a (*.a)")) + self.assertEqual(dlg.call_args[0][2], "baz") + self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*);;c (*.*)") + instance.selectNameFilter.assert_called_with("b (*.*)") + + instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected + self.assertEqual(widget.get_save_filename(), ("", "")) + + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_save_file_dialog_enforces_extension_darwin(self, dlg): + widget = self.widget + for filter1 in widget.filters: + if OWSave._extension_from_filter(filter1) == ".tab": + break + for filter2 in widget.filters: + if OWSave._extension_from_filter(filter2) == ".csv.gz": + break + + widget.filter = filter1 + instance = dlg.return_value + instance.exec.return_value = QFileDialog.Accepted + + instance.selectedNameFilter.return_value = self._mac_filter(filter1) + instance.selectedFiles.return_value = ["foo"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.pkl"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.tab.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.csv.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.tab") + instance.selectedFiles.return_value = ["foo.bar"] + self.assertEqual(widget.get_save_filename()[0], "foo.bar.tab") + + instance.selectedNameFilter.return_value = self._mac_filter(filter2) + instance.selectedFiles.return_value = ["foo"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.pkl"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.tab.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.csv.gz"] + self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") + instance.selectedFiles.return_value = ["foo.bar"] + self.assertEqual(widget.get_save_filename()[0], "foo.bar.csv.gz") + + @patch("Orange.widgets.data.owsave.QFileDialog") + @patch("os.path.exists", new=lambda x: x == "old.tab") + @patch("Orange.widgets.data.owsave.QMessageBox") + def test_save_file_dialog_asks_for_overwrite_darwin(self, msgbox, dlg): + def selected_files(): + nonlocal attempts + attempts += 1 + return [["old.tab", "new.tab"][attempts]] + + widget = self.widget + widget._initial_start_dir = lambda: "baz" + for filter1 in widget.filters: + if OWSave._extension_from_filter(filter1) == ".tab": + break + + widget.filter = filter1 + instance = dlg.return_value + instance.exec.return_value = QFileDialog.Accepted + instance.selectedFiles = selected_files + instance.selectedNameFilter.return_value = self._mac_filter(filter1) + + attempts = -1 + msgbox.question.return_value = msgbox.Yes = 1 + self.assertEqual(widget.get_save_filename()[0], "old.tab") + + attempts = -1 + msgbox.question.return_value = msgbox.No = 0 + self.assertEqual(widget.get_save_filename()[0], "new.tab") + + @patch("Orange.widgets.data.owsave.QFileDialog") + def test_save_file_dialog_uses_valid_filters_darwin(self, dlg): + widget = self.widget + widget._valid_filters = lambda: ["a (*.a)", "b (*.b)"] + widget._default_valid_filter = lambda: "a (*.a)" + instance = dlg.return_value + instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected + widget.get_save_filename() + self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*)") + instance.selectNameFilter.assert_called_with("a (*.*)") + + +@unittest.skipUnless(sys.platform == "win32", "Test for dialog for Windows") +class TestOWSaveWindowsDialog(TestOWSaveBase): # pragma: no cover + @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") + def test_save_file_dialog_uses_valid_filters_windows(self, dlg): + widget = self.widget + widget._valid_filters = lambda: ["a (*.a)", "b (*.b)"] + widget._default_valid_filter = lambda: "a (*.a)" + widget.get_save_filename() + call_args = dlg.call_args + self.assertEqual(call_args[0][3], "a (*.a);;b (*.b)") + self.assertEqual(call_args[0][4], "a (*.a)") + + class TestOWSaveUtils(unittest.TestCase): def test_replace_extension(self): replace = OWSave._replace_extension @@ -524,5 +592,6 @@ def test_extension_from_filter(self): OWSave._extension_from_filter("Description (.foo.bar)"), ".foo.bar") + if __name__ == "__main__": unittest.main() From f482387fd8b17c161722c1f076e5927bdbdfc3cc Mon Sep 17 00:00:00 2001 From: janezd Date: Fri, 1 Mar 2019 10:30:01 +0100 Subject: [PATCH 12/12] OWSave: Use the same dialog for Windows and macOS --- Orange/widgets/data/owsave.py | 37 +++++------ Orange/widgets/data/tests/test_owsave.py | 85 +++++++++++------------- 2 files changed, 55 insertions(+), 67 deletions(-) diff --git a/Orange/widgets/data/owsave.py b/Orange/widgets/data/owsave.py index b79805d2ca6..bb95c4b2f14 100644 --- a/Orange/widgets/data/owsave.py +++ b/Orange/widgets/data/owsave.py @@ -215,31 +215,33 @@ def _default_valid_filter(self): # As of Qt 5.9, QFileDialog.setDefaultSuffix does not support double # suffixes, not even in non-native dialogs. We handle each OS separately. - if sys.platform == "darwin": - # On macOS, is double suffixes are passed to the dialog, they are - # appended multiple times even if already present (QTBUG-44227). - # The only known workaround with native dialog is to use suffix *.*. - # We add the correct suffix after closing the dialog and only then check + if sys.platform in ("darwin", "win32"): + # macOS and Windows native dialogs do not correctly handle double + # extensions. We thus don't pass any suffixes to the dialog and add + # the correct suffix after closing the dialog and only then check # if the file exists and ask whether to override. # It is a bit confusing that the user does not see the final name in the # dialog, but I see no better solution. def get_save_filename(self): # pragma: no cover - def no_suffix(filt): - return filt.split("(")[0] + "(*.*)" + if sys.platform == "darwin": + def remove_star(filt): + return filt.replace(" (*.", " (.") + else: + def remove_star(filt): + return filt - mac_filters = {no_suffix(f): f for f in self._valid_filters()} + no_ext_filters = {remove_star(f): f for f in self._valid_filters()} filename = self._initial_start_dir() while True: dlg = QFileDialog( - None, "Save File", filename, ";;".join(mac_filters)) + None, "Save File", filename, ";;".join(no_ext_filters)) dlg.setAcceptMode(dlg.AcceptSave) - dlg.selectNameFilter(no_suffix(self._default_valid_filter())) - dlg.setOption(QFileDialog.HideNameFilterDetails) + dlg.selectNameFilter(remove_star(self._default_valid_filter())) dlg.setOption(QFileDialog.DontConfirmOverwrite) if dlg.exec() == QFileDialog.Rejected: return "", "" filename = dlg.selectedFiles()[0] - selected_filter = mac_filters[dlg.selectedNameFilter()] + selected_filter = no_ext_filters[dlg.selectedNameFilter()] filename = self._replace_extension( filename, self._extension_from_filter(selected_filter)) if not os.path.exists(filename) or QMessageBox.question( @@ -248,15 +250,6 @@ def no_suffix(filt): "Overwrite?") == QMessageBox.Yes: return filename, selected_filter - elif sys.platform == "win32": - # TODO: Behaviour of getSaveFileName on Windows is not tested!!! - # Windows native dialog may work correctly; if not, we may do the same - # as for macOS? - def get_save_filename(self): # pragma: no cover - return QFileDialog.getSaveFileName( - self, "Save File", self._initial_start_dir(), - ";;".join(self._valid_filters()), self._default_valid_filter()) - else: # Linux and any unknown platforms # Qt does not use a native dialog on Linux, so we can connect to # filterSelected and to overload selectFile to change the extension @@ -279,7 +272,7 @@ def updateDefaultExtension(self, selected_filter): self.suffix = OWSave._extension_from_filter(selected_filter) files = self.selectedFiles() if files and not os.path.isdir(files[0]): - self.selectFile(files[0].split(".")[0]) + self.selectFile(files[0]) def selectFile(self, filename): filename = OWSave._replace_extension(filename, self.suffix) diff --git a/Orange/widgets/data/tests/test_owsave.py b/Orange/widgets/data/tests/test_owsave.py index 031eca9fad2..edc909f29f2 100644 --- a/Orange/widgets/data/tests/test_owsave.py +++ b/Orange/widgets/data/tests/test_owsave.py @@ -3,7 +3,6 @@ from unittest.mock import patch, Mock import os import sys -import re import scipy.sparse as sp from AnyQt.QtWidgets import QFileDialog @@ -410,27 +409,26 @@ def test_get_save_filename_linux(self): def test_save_file_dialog_enforces_extension_linux(self): dialog = OWSave.SaveFileDialog( - None, "Save File", "high.txt", - "Bar files (*.bar);;Low files (*.low)") + None, "Save File", "foo.bar", + "Bar files (*.tab);;Low files (*.csv)") - dialog.selectNameFilter("Low files (*.low)") - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + dialog.selectNameFilter("Low files (*.csv)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/foo.csv")) - dialog.selectFile("high.txt") - print(dialog.selectedFiles()[0]) - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.txt.low")) + dialog.selectFile("high.bar") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar.csv")) - dialog.selectNameFilter("Bar files (*.bar)") - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar")) + dialog.selectNameFilter("Bar files (*.tab)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar.tab")) dialog.selectFile("middle.pkl") - self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.bar")) + self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.tab")) - dialog.filterSelected.emit("Low files (*.low)") - self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.low")) + dialog.filterSelected.emit("Low files (*.csv)") + self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.csv")) dialog.selectFile("high.tab.gz") - self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low")) + self.assertTrue(dialog.selectedFiles()[0].endswith("/high.csv")) def test_save_file_dialog_uses_valid_filters_linux(self): widget = self.widget @@ -444,26 +442,35 @@ def test_save_file_dialog_uses_valid_filters_linux(self): instance.selectNameFilter.assert_called_with("a (*.a)") -@unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS") +@unittest.skipUnless(sys.platform in ("darwin", "win32"), + "Test for native dialog on Windows and macOS") class TestOWSaveDarwinDialog(TestOWSaveBase): # pragma: no cover - @staticmethod - def _mac_filter(filt): - return re.sub(r"\(\*\..*\)", "(*.*)", filt) + if sys.platform == "darwin": + @staticmethod + def remove_star(filt): + return filt.replace(" (*.", " (.") + else: + @staticmethod + def remove_star(filt): + return filt @patch("Orange.widgets.data.owsave.QFileDialog") def test_get_save_filename_darwin(self, dlg): widget = self.widget widget._initial_start_dir = lambda: "baz" - widget.filters = dict.fromkeys(("a (*.a)", "b (*.b)", "c (*.c)")) - widget.filter = "b (*.b)" + widget.filters = dict.fromkeys(("aa (*.a)", "bb (*.b)", "cc (*.c)")) + widget.filter = "bb (*.b)" instance = dlg.return_value instance.exec.return_value = dlg.Accepted = QFileDialog.Accepted instance.selectedFiles.return_value = ["foo"] - instance.selectedNameFilter.return_value = "a (*.*)" - self.assertEqual(widget.get_save_filename(), ("foo.a", "a (*.a)")) + instance.selectedNameFilter.return_value = self.remove_star("aa (*.a)") + self.assertEqual(widget.get_save_filename(), ("foo.a", "aa (*.a)")) self.assertEqual(dlg.call_args[0][2], "baz") - self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*);;c (*.*)") - instance.selectNameFilter.assert_called_with("b (*.*)") + self.assertEqual( + dlg.call_args[0][3], + self.remove_star("aa (*.a);;bb (*.b);;cc (*.c)")) + instance.selectNameFilter.assert_called_with( + self.remove_star("bb (*.b)")) instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected self.assertEqual(widget.get_save_filename(), ("", "")) @@ -482,7 +489,7 @@ def test_save_file_dialog_enforces_extension_darwin(self, dlg): instance = dlg.return_value instance.exec.return_value = QFileDialog.Accepted - instance.selectedNameFilter.return_value = self._mac_filter(filter1) + instance.selectedNameFilter.return_value = self.remove_star(filter1) instance.selectedFiles.return_value = ["foo"] self.assertEqual(widget.get_save_filename()[0], "foo.tab") instance.selectedFiles.return_value = ["foo.pkl"] @@ -494,7 +501,7 @@ def test_save_file_dialog_enforces_extension_darwin(self, dlg): instance.selectedFiles.return_value = ["foo.bar"] self.assertEqual(widget.get_save_filename()[0], "foo.bar.tab") - instance.selectedNameFilter.return_value = self._mac_filter(filter2) + instance.selectedNameFilter.return_value = self.remove_star(filter2) instance.selectedFiles.return_value = ["foo"] self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz") instance.selectedFiles.return_value = ["foo.pkl"] @@ -525,7 +532,7 @@ def selected_files(): instance = dlg.return_value instance.exec.return_value = QFileDialog.Accepted instance.selectedFiles = selected_files - instance.selectedNameFilter.return_value = self._mac_filter(filter1) + instance.selectedNameFilter.return_value = self.remove_star(filter1) attempts = -1 msgbox.question.return_value = msgbox.Yes = 1 @@ -538,26 +545,15 @@ def selected_files(): @patch("Orange.widgets.data.owsave.QFileDialog") def test_save_file_dialog_uses_valid_filters_darwin(self, dlg): widget = self.widget - widget._valid_filters = lambda: ["a (*.a)", "b (*.b)"] - widget._default_valid_filter = lambda: "a (*.a)" + widget._valid_filters = lambda: ["aa (*.a)", "bb (*.b)"] + widget._default_valid_filter = lambda: "aa (*.a)" instance = dlg.return_value instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected widget.get_save_filename() - self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*)") - instance.selectNameFilter.assert_called_with("a (*.*)") - - -@unittest.skipUnless(sys.platform == "win32", "Test for dialog for Windows") -class TestOWSaveWindowsDialog(TestOWSaveBase): # pragma: no cover - @patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName") - def test_save_file_dialog_uses_valid_filters_windows(self, dlg): - widget = self.widget - widget._valid_filters = lambda: ["a (*.a)", "b (*.b)"] - widget._default_valid_filter = lambda: "a (*.a)" - widget.get_save_filename() - call_args = dlg.call_args - self.assertEqual(call_args[0][3], "a (*.a);;b (*.b)") - self.assertEqual(call_args[0][4], "a (*.a)") + self.assertEqual( + dlg.call_args[0][3], self.remove_star("aa (*.a);;bb (*.b)")) + instance.selectNameFilter.assert_called_with( + self.remove_star("aa (*.a)")) class TestOWSaveUtils(unittest.TestCase): @@ -592,6 +588,5 @@ def test_extension_from_filter(self): OWSave._extension_from_filter("Description (.foo.bar)"), ".foo.bar") - if __name__ == "__main__": unittest.main()