Repository: frictionlessdata/tabulator-py (python)

Issue:
Get sheet in Excel/OpenOffice by name
Is there a way to get a sheet by name rather than number?

Pull request:
Support reading XLS/XLSX/ODS by sheet name 
- fixes #195

Patch:
diff --git a/README.md b/README.md
index 4d50158..ab07f9f 100644
--- a/README.md
+++ b/README.md
@@ -357,7 +357,7 @@ Operations:
 - read
 
 Options:
-- sheet - sheet number starting from 1
+- sheet - sheet number starting from 1 OR sheet name
 
 #### sql
 
@@ -399,7 +399,7 @@ Operations:
 - read
 
 Options:
-- sheet - sheet number starting from 1
+- sheet - sheet number starting from 1 OR sheet name
 - fill_merged_cells - if `True` it will unmerge and fill all merged cells by a visible value. With this option enabled the parser can't stream data and load the whole document into memory.
 
 ### Headers
diff --git a/pylama.ini b/pylama.ini
index a13c7b5..64d539a 100644
--- a/pylama.ini
+++ b/pylama.ini
@@ -3,7 +3,7 @@ linters = pyflakes,mccabe,pep8
 ignore = E128, E301,E306,E731
 
 [pylama:pep8]
-max_line_length = 90
+max_line_length = 100
 
 [pylama:mccabe]
 complexity = 16
diff --git a/tabulator/parsers/ods.py b/tabulator/parsers/ods.py
index 7d8c92f..ba336cf 100644
--- a/tabulator/parsers/ods.py
+++ b/tabulator/parsers/ods.py
@@ -4,9 +4,11 @@ from __future__ import print_function
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
+import six
 import ezodf
 from six import BytesIO
 from ..parser import Parser
+from .. import exceptions
 from .. import helpers
 
 
@@ -14,11 +16,6 @@ from .. import helpers
 
 class ODSParser(Parser):
     """Parser to parse ODF Spreadsheets.
-
-    Args:
-        sheet (int or str): sheet number or name
-            First sheet's number is 1.
-
     """
 
     # Public
@@ -29,7 +26,7 @@ class ODSParser(Parser):
 
     def __init__(self, loader, force_parse=False, sheet=1):
         self.__loader = loader
-        self.__index = sheet - 1 if isinstance(sheet, int) else sheet
+        self.__sheet_pointer = sheet
         self.__force_parse = force_parse
         self.__extended_rows = None
         self.__encoding = None
@@ -43,11 +40,24 @@ class ODSParser(Parser):
 
     def open(self, source, encoding=None):
         self.close()
+        self.__encoding = encoding
         self.__bytes = self.__loader.load(
             source, mode='b', encoding=encoding, allow_zip=True)
+
+        # Get book
         self.__book = ezodf.opendoc(BytesIO(self.__bytes.read()))
-        self.__sheet = self.__book.sheets[self.__index]
-        self.__encoding = encoding
+
+        # Get sheet
+        try:
+            if isinstance(self.__sheet_pointer, six.string_types):
+                self.__sheet = self.__book.sheets[self.__sheet_pointer]
+            else:
+                self.__sheet = self.__book.sheets[self.__sheet_pointer - 1]
+        except (KeyError, IndexError):
+            message = 'OpenOffice document "%s" doesn\'t have a sheet "%s"'
+            raise exceptions.SourceError(message % (source, self.__sheet_pointer))
+
+        # Rest parser
         self.reset()
 
     def close(self):
diff --git a/tabulator/parsers/xls.py b/tabulator/parsers/xls.py
index 6c2c210..91d0eef 100644
--- a/tabulator/parsers/xls.py
+++ b/tabulator/parsers/xls.py
@@ -4,8 +4,10 @@ from __future__ import print_function
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
+import six
 import xlrd
 from ..parser import Parser
+from .. import exceptions
 from .. import helpers
 
 
@@ -24,7 +26,7 @@ class XLSParser(Parser):
 
     def __init__(self, loader, force_parse=False, sheet=1, fill_merged_cells=False):
         self.__loader = loader
-        self.__index = sheet - 1
+        self.__sheet_pointer = sheet
         self.__fill_merged_cells = fill_merged_cells
         self.__force_parse = force_parse
         self.__extended_rows = None
@@ -37,13 +39,26 @@ class XLSParser(Parser):
 
     def open(self, source, encoding=None):
         self.close()
+        self.__encoding = encoding
         self.__bytes = self.__loader.load(source, mode='b', encoding=encoding)
+
+        # Get book
         self.__book = xlrd.open_workbook(
-                file_contents=self.__bytes.read(),
-                encoding_override=encoding,
-                formatting_info=True)
-        self.__sheet = self.__book.sheet_by_index(self.__index)
-        self.__encoding = encoding
+            file_contents=self.__bytes.read(),
+            encoding_override=encoding,
+            formatting_info=True)
+
+        # Get sheet
+        try:
+            if isinstance(self.__sheet_pointer, six.string_types):
+                self.__sheet = self.__book.sheet_by_name(self.__sheet_pointer)
+            else:
+                self.__sheet = self.__book.sheet_by_index(self.__sheet_pointer - 1)
+        except (xlrd.XLRDError, IndexError):
+            message = 'Excel document "%s" doesn\'t have a sheet "%s"'
+            raise exceptions.SourceError(message % (source, self.__sheet_pointer))
+
+        # Reset parser
         self.reset()
 
     def close(self):
diff --git a/tabulator/parsers/xlsx.py b/tabulator/parsers/xlsx.py
index 1679024..d166485 100644
--- a/tabulator/parsers/xlsx.py
+++ b/tabulator/parsers/xlsx.py
@@ -4,11 +4,13 @@ from __future__ import print_function
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
+import six
 import shutil
 import openpyxl
 from itertools import chain
 from tempfile import TemporaryFile
 from ..parser import Parser
+from .. import exceptions
 from .. import helpers
 
 
@@ -27,7 +29,7 @@ class XLSXParser(Parser):
 
     def __init__(self, loader, force_parse=False, sheet=1, fill_merged_cells=False):
         self.__loader = loader
-        self.__index = sheet - 1
+        self.__sheet_pointer = sheet
         self.__fill_merged_cells = fill_merged_cells
         self.__extended_rows = None
         self.__encoding = None
@@ -40,8 +42,10 @@ class XLSXParser(Parser):
 
     def open(self, source, encoding=None):
         self.close()
-        self.__bytes = self.__loader.load(
-            source, mode='b', encoding=encoding, allow_zip=True)
+        self.__encoding = encoding
+        self.__bytes = self.__loader.load(source, mode='b', encoding=encoding, allow_zip=True)
+
+        # Create copy for remote source
         # For remote stream we need local copy (will be deleted on close by Python)
         # https://docs.python.org/3.5/library/tempfile.html#tempfile.TemporaryFile
         if hasattr(self.__bytes, 'url'):
@@ -50,13 +54,25 @@ class XLSXParser(Parser):
             self.__bytes.close()
             self.__bytes = new_bytes
             self.__bytes.seek(0)
+
+        # Get book
         # To fill merged cells we can't use read-only because
         # `sheet.merged_cell_ranges` is not available in this mode
         self.__book = openpyxl.load_workbook(
             self.__bytes, read_only=not self.__fill_merged_cells, data_only=True)
-        self.__sheet = self.__book.worksheets[self.__index]
+
+        # Get sheet
+        try:
+            if isinstance(self.__sheet_pointer, six.string_types):
+                self.__sheet = self.__book.get_sheet_by_name(self.__sheet_pointer)
+            else:
+                self.__sheet = self.__book.worksheets[self.__sheet_pointer - 1]
+        except (KeyError, IndexError):
+            message = 'Excel document "%s" doesn\'t have a sheet "%s"'
+            raise exceptions.SourceError(message % (source, self.__sheet_pointer))
         self.__process_merged_cells()
-        self.__encoding = encoding
+
+        # Reset parser
         self.reset()
 
     def close(self):


Test patch:
diff --git a/tests/formats/test_ods.py b/tests/formats/test_ods.py
index bda8659..f8c9677 100644
--- a/tests/formats/test_ods.py
+++ b/tests/formats/test_ods.py
@@ -29,6 +29,28 @@ def test_stream_ods_remote():
         assert stream.read() == [['id', 'name'], [1.0, 'english'], [2.0, '中国人']]
 
 
+def test_stream_ods_sheet_by_index():
+    with Stream('data/table.ods', sheet=1) as stream:
+        assert stream.read() == [['id', 'name'], [1.0, 'english'], [2.0, '中国人']]
+
+
+def test_stream_ods_sheet_by_index_not_existent():
+    with pytest.raises(exceptions.SourceError) as excinfo:
+        Stream('data/table.ods', sheet=3).open()
+    assert 'sheet "3"' in str(excinfo.value)
+
+
+def test_stream_ods_sheet_by_name():
+    with Stream('data/table.ods', sheet='Лист1') as stream:
+        assert stream.read() == [['id', 'name'], [1.0, 'english'], [2.0, '中国人']]
+
+
+def test_stream_ods_sheet_by_index_not_existent():
+    with pytest.raises(exceptions.SourceError) as excinfo:
+        Stream('data/table.ods', sheet='not-existent').open()
+    assert 'sheet "not-existent"' in str(excinfo.value)
+
+
 # Parser
 
 def test_parser_ods():
diff --git a/tests/formats/test_xls.py b/tests/formats/test_xls.py
index 31d58c4..1187c9d 100644
--- a/tests/formats/test_xls.py
+++ b/tests/formats/test_xls.py
@@ -5,6 +5,7 @@ from __future__ import absolute_import
 from __future__ import unicode_literals
 
 import io
+import pytest
 from mock import Mock
 from tabulator import parsers
 from tabulator import Stream, exceptions
@@ -26,12 +27,32 @@ def test_stream_remote_xls():
         assert stream.read() == [['id', 'name'], [1.0, 'english'], [2.0, '中国人']]
 
 
-def test_stream_xls_sheet():
+def test_stream_xls_sheet_by_index():
     source = 'data/special/sheet2.xls'
     with Stream(source, sheet=2) as stream:
         assert stream.read() == [['id', 'name'], [1.0, 'english'], [2.0, '中国人']]
 
 
+def test_stream_xls_sheet_by_index_not_existent():
+    source = 'data/special/sheet2.xls'
+    with pytest.raises(exceptions.SourceError) as excinfo:
+        Stream(source, sheet=3).open()
+    assert 'sheet "3"' in str(excinfo.value)
+
+
+def test_stream_xls_sheet_by_name():
+    source = 'data/special/sheet2.xls'
+    with Stream(source, sheet='Sheet2') as stream:
+        assert stream.read() == [['id', 'name'], [1.0, 'english'], [2.0, '中国人']]
+
+
+def test_stream_xls_sheet_by_name_not_existent():
+    source = 'data/special/sheet2.xls'
+    with pytest.raises(exceptions.SourceError) as excinfo:
+        Stream(source, sheet='not-existent').open()
+    assert 'sheet "not-existent"' in str(excinfo.value)
+
+
 def test_stream_xlsx_merged_cells():
     source = 'data/special/merged-cells.xls'
     with Stream(source) as stream:
diff --git a/tests/formats/test_xlsx.py b/tests/formats/test_xlsx.py
index 55dcb9b..0b87a38 100644
--- a/tests/formats/test_xlsx.py
+++ b/tests/formats/test_xlsx.py
@@ -5,6 +5,7 @@ from __future__ import absolute_import
 from __future__ import unicode_literals
 
 import io
+import pytest
 from mock import Mock
 from tabulator import Stream, exceptions
 from tabulator.parsers.xlsx import XLSXParser
@@ -26,12 +27,32 @@ def test_stream_xlsx_stream():
         assert stream.read() == [['id', 'name'], [1.0, 'english'], [2.0, '中国人']]
 
 
-def test_stream_xlsx_sheet():
+def test_stream_xlsx_sheet_by_index():
     source = 'data/special/sheet2.xlsx'
     with Stream(source, sheet=2) as stream:
         assert stream.read() == [['id', 'name'], [1, 'english'], [2, '中国人']]
 
 
+def test_stream_xlsx_sheet_by_index_not_existent():
+    source = 'data/special/sheet2.xlsx'
+    with pytest.raises(exceptions.SourceError) as excinfo:
+        Stream(source, sheet=3).open()
+    assert 'sheet "3"' in str(excinfo.value)
+
+
+def test_stream_xlsx_sheet_by_name():
+    source = 'data/special/sheet2.xlsx'
+    with Stream(source, sheet='Sheet2') as stream:
+        assert stream.read() == [['id', 'name'], [1, 'english'], [2, '中国人']]
+
+
+def test_stream_xlsx_sheet_by_name_not_existent():
+    source = 'data/special/sheet2.xlsx'
+    with pytest.raises(exceptions.SourceError) as excinfo:
+        Stream(source, sheet='not-existent').open()
+    assert 'sheet "not-existent"' in str(excinfo.value)
+
+
 def test_stream_xlsx_merged_cells():
     source = 'data/special/merged-cells.xlsx'
     with Stream(source) as stream: