Merge pull request #5736 from markotoplak/owfile-reader

[ENH] File: explicit file format choice

Author: VesnaT

Orange/data/io_base.py

@@ -31,6 +31,11 @@
 PICKLE_PROTOCOL = 4
 
 
+class MissingReaderException(IOError):
+    # subclasses IOError for backward compatibility
+    pass
+
+
 class Flags:
     """Parser for column flags (i.e. third header row)"""
     DELIMITER = ' '
@@ -551,7 +556,7 @@ def get_reader(cls, filename):
             if fnmatch(path.basename(filename), '*' + ext):
                 return reader(filename)
 
-        raise IOError('No readers for file "{}"'.format(filename))
+        raise MissingReaderException('No readers for file "{}"'.format(filename))
 
     @classmethod
     def set_table_metadata(cls, filename, table):

Orange/widgets/data/owfile.py

@@ -9,11 +9,14 @@
     QStyle, QComboBox, QMessageBox, QGridLayout, QLabel, \
     QLineEdit, QSizePolicy as Policy, QCompleter
 from AnyQt.QtCore import Qt, QTimer, QSize, QUrl
+from AnyQt.QtGui import QBrush
 
+from orangewidget.utils.filedialogs import format_filter
 from orangewidget.workflow.drophandler import SingleUrlDropHandler
 
 from Orange.data.table import Table, get_sample_datasets_dir
 from Orange.data.io import FileFormat, UrlReader, class_from_qualified_name
+from Orange.data.io_base import MissingReaderException
 from Orange.util import log_warnings
 from Orange.widgets import widget, gui
 from Orange.widgets.settings import Setting, ContextSetting, \
@@ -30,6 +33,8 @@
 # module's namespace so that old saved settings still work
 from Orange.widgets.utils.filedialogs import RecentPath
 
+DEFAULT_READER_TEXT = "Automatically detect type"
+
 log = logging.getLogger(__name__)
 
 
@@ -121,6 +126,9 @@ class Outputs:
 
     domain_editor = SettingProvider(DomainEditor)
 
+    class Information(widget.OWWidget.Information):
+        no_file_selected = Msg("No file selected.")
+
     class Warning(widget.OWWidget.Warning):
         file_too_big = Msg("The file is too large to load automatically."
                            " Press Reload to load.")
@@ -137,9 +145,6 @@ class Error(widget.OWWidget.Error):
         sheet_error = Msg("Error listing available sheets.")
         unknown = Msg("Read error:\n{}")
 
-    class NoFileSelected:
-        pass
-
     UserAdviceMessages = [
         widget.Message(
             "Use CSV File Import widget for advanced options "
@@ -160,6 +165,23 @@ def __init__(self):
         self.loaded_file = ""
         self.reader = None
 
+        readers = [f for f in FileFormat.formats
+                   if getattr(f, 'read', None)
+                   and getattr(f, "EXTENSIONS", None)]
+
+        def group_readers_per_addon_key(w):
+            # readers from Orange.data.io should go first
+            def package(w):
+                package = w.qualified_name().split(".")[:-1]
+                package = package[:2]
+                if ".".join(package) == "Orange.data":
+                    return ["0"]  # force "Orange" to come first
+                return package
+            return package(w), w.DESCRIPTION
+
+        self.available_readers = sorted(set(readers),
+                                        key=group_readers_per_addon_key)
+
         layout = QGridLayout()
         layout.setSpacing(4)
         gui.widgetBox(self.controlArea, orientation=layout, box='Source')
@@ -227,6 +249,19 @@ def __init__(self):
         completer.setCaseSensitivity(Qt.CaseSensitive)
         url_combo.setCompleter(completer)
 
+        layout = QGridLayout()
+        layout.setSpacing(4)
+        gui.widgetBox(self.controlArea, orientation=layout, box='File Type')
+
+        box = gui.hBox(None, addToLayout=False, margin=0)
+        box.setSizePolicy(Policy.MinimumExpanding, Policy.Fixed)
+        self.reader_combo = QComboBox(self)
+        self.reader_combo.setSizePolicy(Policy.MinimumExpanding, Policy.Fixed)
+        self.reader_combo.activated[int].connect(self.select_reader)
+
+        box.layout().addWidget(self.reader_combo)
+        layout.addWidget(box, 0, 1)
+
         box = gui.vBox(self.controlArea, "Info")
         self.infolabel = gui.widgetLabel(box, 'No data loaded.')
 
@@ -286,6 +321,23 @@ def select_sheet(self):
         self.recent_paths[0].sheet = self.sheet_combo.currentText()
         self.load_data()
 
+    def select_reader(self, n):
+        if self.source != self.LOCAL_FILE:
+            return  # ignore for URL's
+
+        if self.recent_paths:
+            path = self.recent_paths[0]
+            if n == 0:  # default
+                path.file_format = None
+                self.load_data()
+            elif n <= len(self.available_readers):
+                reader = self.available_readers[n - 1]
+                path.file_format = reader.qualified_name()
+                self.load_data()
+            else:  # the rest include just qualified names
+                path.file_format = self.reader_combo.itemText(n)
+                self.load_data()
+
     def _url_set(self):
         url = self.url_combo.currentText()
         pos = self.recent_urls.index(url)
@@ -310,10 +362,7 @@ def browse_file(self, in_demos=False):
         else:
             start_file = self.last_path() or os.path.expanduser("~/")
 
-        readers = [f for f in FileFormat.formats
-                   if getattr(f, 'read', None)
-                   and getattr(f, "EXTENSIONS", None)]
-        filename, reader, _ = open_filename_dialog(start_file, None, readers)
+        filename, reader, _ = open_filename_dialog(start_file, None, self.available_readers)
         if not filename:
             return
         self.add_path(filename)
@@ -342,19 +391,33 @@ def load_data(self):
             self.infolabel.setText("No data.")
 
     def _try_load(self):
+        self._initialize_reader_combo()
+
         # pylint: disable=broad-except
-        if self.last_path() and not os.path.exists(self.last_path()):
-            return self.Error.file_not_found
+        if self.source == self.LOCAL_FILE:
+            if self.last_path() is None:
+                return self.Information.no_file_selected
+            elif not os.path.exists(self.last_path()):
+                return self.Error.file_not_found
+        else:
+            url = self.url_combo.currentText().strip()
+            if not url:
+                return self.Information.no_file_selected
+
+        def mark_problematic_reader():
+            self.reader_combo.setItemData(self.reader_combo.currentIndex(),
+                                          QBrush(Qt.red), Qt.ForegroundRole)
 
         try:
-            self.reader = self._get_reader()
+            self.reader = self._get_reader()  # also sets current reader index
             assert self.reader is not None
-        except Exception:
+        except MissingReaderException:
+            mark_problematic_reader()
             return self.Error.missing_reader
-
-        if self.reader is self.NoFileSelected:
-            self.Outputs.data.send(None)
-            return None
+        except Exception as ex:
+            mark_problematic_reader()
+            log.exception(ex)
+            return lambda x=ex: self.Error.unknown(str(x))
 
         try:
             self._update_sheet_combo()
@@ -365,6 +428,7 @@ def _try_load(self):
             try:
                 data = self.reader.read()
             except Exception as ex:
+                mark_problematic_reader()
                 log.exception(ex)
                 return lambda x=ex: self.Error.unknown(str(x))
             if warnings:
@@ -382,23 +446,31 @@ def _try_load(self):
     def _get_reader(self) -> FileFormat:
         if self.source == self.LOCAL_FILE:
             path = self.last_path()
-            if path is None:
-                return self.NoFileSelected
+            self.reader_combo.setEnabled(True)
             if self.recent_paths and self.recent_paths[0].file_format:
                 qname = self.recent_paths[0].file_format
-                reader_class = class_from_qualified_name(qname)
+                qname_index = {r.qualified_name(): i for i, r in enumerate(self.available_readers)}
+                if qname in qname_index:
+                    self.reader_combo.setCurrentIndex(qname_index[qname] + 1)
+                else:
+                    # reader may be accessible, but not in self.available_readers
+                    # (perhaps its code was moved)
+                    self.reader_combo.addItem(qname)
+                    self.reader_combo.setCurrentIndex(len(self.reader_combo) - 1)
+                try:
+                    reader_class = class_from_qualified_name(qname)
+                except Exception as ex:
+                    raise MissingReaderException(f'Can not find reader "{qname}"') from ex
                 reader = reader_class(path)
             else:
+                self.reader_combo.setCurrentIndex(0)
                 reader = FileFormat.get_reader(path)
             if self.recent_paths and self.recent_paths[0].sheet:
                 reader.select_sheet(self.recent_paths[0].sheet)
             return reader
         else:
             url = self.url_combo.currentText().strip()
-            if url:
-                return UrlReader(url)
-            else:
-                return self.NoFileSelected
+            return UrlReader(url)
 
     def _update_sheet_combo(self):
         if len(self.reader.sheets) < 2:
@@ -420,6 +492,14 @@ def _select_active_sheet(self):
             self.reader.select_sheet(None)
             self.sheet_combo.setCurrentIndex(0)
 
+    def _initialize_reader_combo(self):
+        self.reader_combo.clear()
+        filters = [format_filter(f) for f in self.available_readers]
+        self.reader_combo.addItems([DEFAULT_READER_TEXT] + filters)
+        self.reader_combo.setCurrentIndex(0)
+        self.reader_combo.setDisabled(True)
+        # additional readers may be added in self._get_reader()
+
     @staticmethod
     def _describe(table):
         def missing_prop(prop):
@@ -551,7 +631,7 @@ def dragEnterEvent(event):
             try:
                 FileFormat.get_reader(urls[0].toLocalFile())
                 event.acceptProposedAction()
-            except IOError:
+            except MissingReaderException:
                 pass
 
     def dropEvent(self, event):

Orange/widgets/data/tests/test_owfile.py

@@ -24,7 +24,7 @@
 
 from Orange.data.io import TabReader
 from Orange.tests import named_file
-from Orange.widgets.data.owfile import OWFile, OWFileDropHandler
+from Orange.widgets.data.owfile import OWFile, OWFileDropHandler, DEFAULT_READER_TEXT
 from Orange.widgets.utils.filedialogs import dialog_formats, format_filter, RecentPath
 from Orange.widgets.tests.base import WidgetTest
 from Orange.widgets.utils.domaineditor import ComboDelegate, VarTypeDelegate, VarTableModel
@@ -253,12 +253,14 @@ def test_nothing_selected(self):
             self.create_widget(OWFile, stored_settings={"recent_paths": []})
 
         widget.Outputs.data.send = Mock()
-        widget._try_load()
+        widget.load_data()
+        self.assertTrue(widget.Information.no_file_selected.is_shown())
         widget.Outputs.data.send.assert_called_with(None)
 
         widget.Outputs.data.send.reset_mock()
         widget.source = widget.URL
-        widget._try_load()
+        widget.load_data()
+        self.assertTrue(widget.Information.no_file_selected.is_shown())
         widget.Outputs.data.send.assert_called_with(None)
 
     def test_check_column_noname(self):
@@ -380,6 +382,7 @@ def open_iris_with_no_spec_format(_a, _b, _c, filters, _e):
             self.widget.browse_file()
 
         self.assertIsNone(self.widget.recent_paths[0].file_format)
+        self.assertEqual(self.widget.reader_combo.currentText(), DEFAULT_READER_TEXT)
 
         def open_iris_with_tab(*_):
             return iris.__file__, format_filter(TabReader)
@@ -389,6 +392,7 @@ def open_iris_with_tab(*_):
             self.widget.browse_file()
 
         self.assertEqual(self.widget.recent_paths[0].file_format, "Orange.data.io.TabReader")
+        self.assertTrue(self.widget.reader_combo.currentText().startswith("Tab-separated"))
 
     def test_no_specified_reader(self):
         with named_file("", suffix=".tab") as fn:
@@ -397,6 +401,52 @@ def test_no_specified_reader(self):
                                              stored_settings={"recent_paths": [no_class]})
             self.widget.load_data()
         self.assertTrue(self.widget.Error.missing_reader.is_shown())
+        self.assertEqual(self.widget.reader_combo.currentText(), "not.a.file.reader.class")
+
+    def test_select_reader(self):
+        filename = FileFormat.locate("iris.tab", dataset_dirs)
+
+        # a setting which adds a new qualified name to the reader combo
+        no_class = RecentPath(filename, None, None, file_format="not.a.file.reader.class")
+        self.widget = self.create_widget(OWFile,
+                                         stored_settings={"recent_paths": [no_class]})
+        self.widget.load_data()
+        len_with_qname = len(self.widget.reader_combo)
+        self.assertEqual(self.widget.reader_combo.currentText(), "not.a.file.reader.class")
+        self.assertEqual(self.widget.reader, None)
+
+        # select the last option, the same reader
+        self.widget.reader_combo.activated.emit(len_with_qname - 1)
+        self.assertEqual(len(self.widget.reader_combo), len_with_qname)
+        self.assertEqual(self.widget.reader_combo.currentText(), "not.a.file.reader.class")
+        self.assertEqual(self.widget.reader, None)
+
+        # select the tab reader
+        for i in range(len_with_qname):
+            text = self.widget.reader_combo.itemText(i)
+            if text.startswith("Tab-separated"):
+                break
+        self.widget.reader_combo.activated.emit(i)
+        self.assertEqual(len(self.widget.reader_combo), len_with_qname - 1)
+        self.assertTrue(self.widget.reader_combo.currentText().startswith("Tab-separated"))
+        self.assertIsInstance(self.widget.reader, TabReader)
+
+        # select the default reader
+        self.widget.reader_combo.activated.emit(0)
+        self.assertEqual(len(self.widget.reader_combo), len_with_qname - 1)
+        self.assertEqual(self.widget.reader_combo.currentText(), DEFAULT_READER_TEXT)
+        self.assertIsInstance(self.widget.reader, TabReader)
+
+    def test_select_reader_errors(self):
+        filename = FileFormat.locate("iris.tab", dataset_dirs)
+
+        no_class = RecentPath(filename, None, None, file_format="Orange.data.io.ExcelReader")
+        self.widget = self.create_widget(OWFile,
+                                         stored_settings={"recent_paths": [no_class]})
+        self.widget.load_data()
+        self.assertIn("Excel", self.widget.reader_combo.currentText())
+        self.assertTrue(self.widget.Error.unknown.is_shown())
+        self.assertFalse(self.widget.Error.missing_reader.is_shown())
 
     def test_domain_edit_no_changes(self):
         self.open_dataset("iris")
@@ -408,12 +458,12 @@ def test_domain_edit_no_changes(self):
     def test_domain_edit_on_sparse_data(self):
         iris = Table("iris").to_sparse()
 
-        f = tempfile.NamedTemporaryFile(suffix='.pickle', delete=False)
-        pickle.dump(iris, f)
-        f.close()
+        with named_file("", suffix='.pickle') as fn:
+            with open(fn, "wb") as f:
+                pickle.dump(iris, f)
 
-        self.widget.add_path(f.name)
-        self.widget.load_data()
+            self.widget.add_path(fn)
+            self.widget.load_data()
 
         output = self.get_output(self.widget.Outputs.data)
         self.assertIsInstance(output, Table)
@@ -552,9 +602,8 @@ def test_open_moved_workflow(self):
         (i.e. sent by email), considering data file is stored in the same
         directory as the workflow.
         """
-        temp_file = tempfile.NamedTemporaryFile(dir=getcwd(), delete=False)
-        file_name = temp_file.name
-        temp_file.close()
+        with tempfile.NamedTemporaryFile(dir=getcwd(), delete=False) as temp_file:
+            file_name = temp_file.name
         base_name = path.basename(file_name)
         try:
             recent_path = RecentPath(
@@ -575,9 +624,8 @@ def test_files_relocated(self):
         """
         This test testes if paths are relocated correctly
         """
-        temp_file = tempfile.NamedTemporaryFile(dir=getcwd(), delete=False)
-        file_name = temp_file.name
-        temp_file.close()
+        with tempfile.NamedTemporaryFile(dir=getcwd(), delete=False) as temp_file:
+            file_name = temp_file.name
         base_name = path.basename(file_name)
         try:
             recent_path = RecentPath(