Merge pull request #4279 from VesnaT/xlsx_reader

[FIX] ExcelReader: Migrate to openpyxl

Author: janezd

MANIFEST.in

@@ -2,7 +2,7 @@
 recursive-include Orange *.pyx *.py *.c *.cpp README* LICENSE
 recursive-include Orange/datasets *.tab *.basket *.info *.dst *.metadata
 
-recursive-include Orange/tests *.tab *.basket *.xlsx *.pkl *.pkl.gz
+recursive-include Orange/tests *.tab *.basket *.xlsx *.xls *.pkl *.pkl.gz
 
 recursive-include Orange/canvas *ico *.png *.svg *.ico
 recursive-include Orange/canvas/workflows *.ows

Orange/data/io.py

@@ -6,6 +6,7 @@
 import subprocess
 import sys
 import warnings
+from typing import List, Iterable
 
 from ast import literal_eval
 from collections import OrderedDict, Counter, defaultdict
@@ -26,6 +27,7 @@
 
 import xlrd
 import xlsxwriter
+import openpyxl
 
 from Orange.data import (
     _io, is_discrete_values, MISSING_VALUES, Table, Domain, Variable,
@@ -405,15 +407,15 @@ def __init__(self, filename):
         self.sheet = None
 
     @property
-    def sheets(self):
+    def sheets(self) -> List:
         """FileFormats with a notion of sheets should override this property
         to return a list of sheet names in the file.
 
         Returns
         -------
         a list of sheet names
         """
-        return ()
+        return []
 
     def select_sheet(self, sheet):
         """Select sheet to be read
@@ -998,44 +1000,21 @@ def constr_vars(inds):
         return table
 
 
-class ExcelReader(FileFormat):
-    """Reader for excel files"""
-    EXTENSIONS = ('.xlsx',)
-    DESCRIPTION = 'Microsoft Excel spreadsheet'
+class _BaseExcelReader(FileFormat):
+    """Base class for reading excel files"""
     SUPPORT_COMPRESSED = False
     SUPPORT_SPARSE_DATA = False
 
     def __init__(self, filename):
         super().__init__(filename=filename)
         self._workbook = None
 
-    @property
-    def workbook(self):
-        if not self._workbook:
-            self._workbook = xlrd.open_workbook(self.filename, on_demand=True)
-        return self._workbook
-
-    @property
-    @lru_cache(1)
-    def sheets(self):
-        if self.workbook:
-            return self.workbook.sheet_names()
-        else:
-            return ()
+    def get_cells(self) -> Iterable:
+        raise NotImplementedError
 
     def read(self):
-        if self.sheet:
-            ss = self.workbook.sheet_by_name(self.sheet)
-        else:
-            ss = self.workbook.sheet_by_index(0)
         try:
-            first_row = next(i for i in range(ss.nrows) if any(ss.row_values(i)))
-            first_col = next(i for i in range(ss.ncols) if ss.cell_value(first_row, i))
-            row_len = ss.row_len(first_row)
-            cells = filter(any,
-                           [[str(ss.cell_value(row, col)) if col < ss.row_len(row) else ''
-                             for col in range(first_col, row_len)]
-                            for row in range(first_row, ss.nrows)])
+            cells = self.get_cells()
             table = self.data_table(cells)
             table.name = path.splitext(path.split(self.filename)[-1])[0]
             if self.sheet:
@@ -1066,6 +1045,74 @@ def write_file(cls, filename, data):
         workbook.close()
 
 
+class ExcelReader(_BaseExcelReader):
+    """Reader for .xlsx files"""
+    EXTENSIONS = ('.xlsx',)
+    DESCRIPTION = 'Microsoft Excel spreadsheet'
+
+    @property
+    def workbook(self) -> openpyxl.Workbook:
+        if not self._workbook:
+            self._workbook = openpyxl.load_workbook(self.filename)
+        return self._workbook
+
+    @property
+    @lru_cache(1)
+    def sheets(self) -> List:
+        return self.workbook.sheetnames if self.workbook else []
+
+    def get_cells(self) -> Iterable:
+        def str_(x):
+            return str(x) if x is not None else ""
+
+        sheet = self._get_active_sheet()
+        cells = ([str_(sheet.cell(row, col).value)
+                  for col in range(sheet.min_column, sheet.max_column + 1)]
+                 for row in range(sheet.min_row, sheet.max_row + 1))
+        return filter(any, cells)
+
+    def _get_active_sheet(self) -> openpyxl.worksheet.worksheet.Worksheet:
+        if self.sheet:
+            return self.workbook[self.sheet]
+        else:
+            return self.workbook.active
+
+
+class XlsReader(_BaseExcelReader):
+    """Reader for .xls files"""
+    EXTENSIONS = ('.xls',)
+    DESCRIPTION = 'Microsoft Excel 97-2004 spreadsheet'
+
+    @property
+    def workbook(self) -> xlrd.Book:
+        if not self._workbook:
+            self._workbook = xlrd.open_workbook(self.filename, on_demand=True)
+        return self._workbook
+
+    @property
+    @lru_cache(1)
+    def sheets(self) -> List:
+        return self.workbook.sheet_names() if self.workbook else []
+
+    def get_cells(self) -> Iterable:
+        sheet = self._get_active_sheet()
+        first_row = next(i for i in range(sheet.nrows)
+                         if any(sheet.row_values(i)))
+        first_col = next(i for i in range(sheet.ncols)
+                         if sheet.cell_value(first_row, i))
+        row_len = sheet.row_len(first_row)
+        return filter(any, ([str(sheet.cell_value(row, col))
+                             if col < sheet.row_len(row) else ''
+                             for col in range(first_col, row_len)]
+                            for row in range(first_row, sheet.nrows)))
+
+    def _get_active_sheet(self) -> xlrd.sheet.Sheet:
+        if self.sheet:
+            return self.workbook.sheet_by_name(self.sheet)
+        else:
+            return self.workbook.sheet_by_index(0)
+
+
 class DotReader(FileFormat):
     """Writer for dot (graph) files"""
     EXTENSIONS = ('.dot', '.gv')

Orange/tests/test_tab_reader.py

@@ -195,7 +195,7 @@ def test_sheets(self):
         file1 = io.StringIO("\n".join("xd dbac"))
         reader = TabReader(file1)
 
-        self.assertEqual(reader.sheets, ())
+        self.assertEqual(reader.sheets, [])
 
     def test_attributes_saving(self):
         tempdir = tempfile.mkdtemp()

Orange/tests/test_xlsx_reader.py

@@ -3,6 +3,8 @@
 
 import unittest
 import os
+from functools import wraps
+from typing import Callable
 
 import numpy as np
 
@@ -13,13 +15,42 @@ def get_dataset(name):
     return os.path.join(os.path.dirname(__file__), "xlsx_files", name)
 
 
-def read_file(name):
-    return io.ExcelReader(get_dataset(name)).read()
+def get_xlsx_reader(name: str) -> io.ExcelReader:
+    return io.ExcelReader(get_dataset(name))
+
+
+def get_xls_reader(name: str) -> io.XlsReader:
+    return io.XlsReader(get_dataset(name))
+
+
+def read_file(reader: Callable, name: str) -> Table:
+    return reader(name).read()
+
+
+def test_xlsx_xls(f):
+    @wraps(f)
+    def wrapper(self):
+        f(self, get_xlsx_reader)
+        f(self, get_xls_reader)
+    return wrapper
+
+
+class TestExcelReader(unittest.TestCase):
+    def test_read_round_floats(self):
+        table = read_file(get_xlsx_reader, "round_floats.xlsx")
+        domain = table.domain
+        self.assertIsNone(domain.class_var)
+        self.assertEqual(len(domain.metas), 0)
+        self.assertEqual(len(domain.attributes), 3)
+        self.assertIsInstance(domain[0], ContinuousVariable)
+        self.assertIsInstance(domain[1], ContinuousVariable)
+        self.assertListEqual(domain[2].values, ["1", "2"])
 
 
 class TestExcelHeader0(unittest.TestCase):
-    def test_read(self):
-        table = read_file("header_0.xlsx")
+    @test_xlsx_xls
+    def test_read(self, reader: Callable[[str], io.FileFormat]):
+        table = read_file(reader, "header_0.xlsx")
         domain = table.domain
         self.assertIsNone(domain.class_var)
         self.assertEqual(len(domain.metas), 0)
@@ -35,29 +66,37 @@ def test_read(self):
 
 
 class TextExcelSheets(unittest.TestCase):
-    def setUp(self):
-        self.reader = io.ExcelReader(get_dataset("header_0_sheet.xlsx"))
-
-    def test_sheets(self):
-        self.assertSequenceEqual(self.reader.sheets,
+    @test_xlsx_xls
+    def test_sheets(self, reader: Callable[[str], io.FileFormat]):
+        reader = reader("header_0_sheet.xlsx")
+        self.assertSequenceEqual(reader.sheets,
                                  ["Sheet1", "my_sheet", "Sheet3"])
 
-    def test_named_sheet(self):
-        self.reader.select_sheet("my_sheet")
-        table = self.reader.read()
+    @test_xlsx_xls
+    def test_named_sheet(self, reader: Callable[[str], io.FileFormat]):
+        reader = reader("header_0_sheet.xlsx")
+        reader.select_sheet("my_sheet")
+        table = reader.read()
         self.assertEqual(len(table.domain.attributes), 4)
         self.assertEqual(table.name, 'header_0_sheet-my_sheet')
 
-    def test_named_sheet_table(self):
+    def test_named_sheet_table_xlsx(self):
         table = Table.from_file(get_dataset("header_0_sheet.xlsx"),
                                 sheet="my_sheet")
         self.assertEqual(len(table.domain.attributes), 4)
         self.assertEqual(table.name, 'header_0_sheet-my_sheet')
 
+    def test_named_sheet_table_xls(self):
+        table = Table.from_file(get_dataset("header_0_sheet.xls"),
+                                sheet="my_sheet")
+        self.assertEqual(len(table.domain.attributes), 4)
+        self.assertEqual(table.name, 'header_0_sheet-my_sheet')
+
 
 class TestExcelHeader1(unittest.TestCase):
-    def test_no_flags(self):
-        table = read_file("header_1_no_flags.xlsx")
+    @test_xlsx_xls
+    def test_no_flags(self, reader: Callable[[str], io.FileFormat]):
+        table = read_file(reader, "header_1_no_flags.xlsx")
         domain = table.domain
         self.assertEqual(len(domain.metas), 0)
         self.assertEqual(len(domain.attributes), 4)
@@ -74,8 +113,9 @@ def test_no_flags(self):
                                                  [0, 0, np.nan, 0]]))
         np.testing.assert_equal(table.Y, np.array([]).reshape(3, 0))
 
-    def test_flags(self):
-        table = read_file("header_1_flags.xlsx")
+    @test_xlsx_xls
+    def test_flags(self, reader: Callable[[str], io.FileFormat]):
+        table = read_file(reader, "header_1_flags.xlsx")
         domain = table.domain
 
         self.assertEqual(len(domain.attributes), 1)
@@ -104,8 +144,9 @@ def test_flags(self):
 
 
 class TestExcelHeader3(unittest.TestCase):
-    def test_read(self):
-        table = read_file("header_3.xlsx")
+    @test_xlsx_xls
+    def test_read(self, reader: Callable[[str], io.FileFormat]):
+        table = read_file(reader, "header_3.xlsx")
         domain = table.domain
 
         self.assertEqual(len(domain.attributes), 2)

Orange/tests/xlsx_files/header_0_sheet.xls

@@ -1,7 +1,8 @@
 import os.path
 
 from Orange.data.table import Table
-from Orange.data.io import TabReader, CSVReader, PickleReader, ExcelReader
+from Orange.data.io import TabReader, CSVReader, PickleReader, ExcelReader, \
+    XlsReader
 from Orange.widgets import gui, widget
 from Orange.widgets.widget import Input
 from Orange.widgets.settings import Setting
@@ -21,7 +22,7 @@ class OWSave(OWSaveBase):
 
     settings_version = 2
 
-    writers = [TabReader, CSVReader, PickleReader, ExcelReader]
+    writers = [TabReader, CSVReader, PickleReader, ExcelReader, XlsReader]
     filters = {
         **{f"{w.DESCRIPTION} (*{w.EXTENSIONS[0]})": w
            for w in writers},

Orange/tests/xlsx_files/round_floats.xlsx

@@ -21,3 +21,4 @@ requests
 openTSNE>=0.3.11
 pandas
 pyyaml
+openpyxl

Orange/widgets/data/owsave.py

@@ -210,6 +210,7 @@ def configuration(parent_package='', top_path=None):
     "Orange.widgets.unsupervised": ["icons/*.svg"],
     "Orange.widgets.utils": ["_webview/*.js"],
     "Orange.tests": ["xlsx_files/*.xlsx", "datasets/*.tab",
+                     "xlsx_files/*.xls",
                      "datasets/*.basket", "datasets/*.csv",
                      "datasets/*.pkl", "datasets/*.pkl.gz"]
 }