FIX: pandas.ExcelFile should not close stream passed as parameter on destruction

Regression test added

Bonus: refactor closing of openpyxl file into openpyxl class instead
of anti-pattern check in ExcelFile.close() for openpyxl engine

Author: roberthdevries

pandas/io/excel/_base.py

@@ -366,6 +366,10 @@ def _workbook_class(self):
     def load_workbook(self, filepath_or_buffer):
         pass
 
+    @abc.abstractmethod
+    def close(self):
+        pass
+
     @property
     @abc.abstractmethod
     def sheet_names(self):
@@ -895,14 +899,7 @@ def sheet_names(self):
 
     def close(self):
         """close io if necessary"""
-        if self.engine == "openpyxl":
-            # https://stackoverflow.com/questions/31416842/
-            #  openpyxl-does-not-close-excel-workbook-in-read-only-mode
-            wb = self.book
-            wb._archive.close()
-
-        if hasattr(self.io, "close"):
-            self.io.close()
+        self._reader.close()
 
     def __enter__(self):
         return self

pandas/io/excel/_odfreader.py

@@ -33,6 +33,9 @@ def load_workbook(self, filepath_or_buffer: FilePathOrBuffer):
 
         return load(filepath_or_buffer)
 
+    def close(self):
+        pass
+
     @property
     def empty_value(self) -> str:
         """Property for compat with other readers."""

pandas/io/excel/_openpyxl.py

@@ -492,6 +492,11 @@ def load_workbook(self, filepath_or_buffer: FilePathOrBuffer):
             filepath_or_buffer, read_only=True, data_only=True, keep_links=False
         )
 
+    def close(self):
+        # https://stackoverflow.com/questions/31416842/
+        #  openpyxl-does-not-close-excel-workbook-in-read-only-mode
+        self.book.close()
+
     @property
     def sheet_names(self) -> List[str]:
         return self.book.sheetnames

pandas/io/excel/_pyxlsb.py

@@ -36,6 +36,9 @@ def load_workbook(self, filepath_or_buffer: FilePathOrBuffer):
 
         return open_workbook(filepath_or_buffer)
 
+    def close(self):
+        pass
+
     @property
     def sheet_names(self) -> List[str]:
         return self.book.sheets

pandas/io/excel/_xlrd.py

@@ -36,6 +36,9 @@ def load_workbook(self, filepath_or_buffer):
         else:
             return open_workbook(filepath_or_buffer)
 
+    def close(self):
+        pass
+
     @property
     def sheet_names(self):
         return self.book.sheet_names()

pandas/tests/io/excel/test_readers.py

@@ -629,6 +629,20 @@ def test_read_from_py_localpath(self, read_ext):
 
         tm.assert_frame_equal(expected, actual)
 
+    @td.skip_if_no("py.path")
+    @td.check_file_leaks
+    def test_close_from_py_localpath(self, read_ext):
+
+        # GH31467
+        from py.path import local as LocalPath
+
+        str_path = os.path.join("test1" + read_ext)
+        with open(str_path, 'rb') as f:
+            x = pd.read_excel(f, "Sheet1", index_col=0)
+            del x
+            # should not throw an exception because the passed file was closed
+            f.read()
+
     def test_reader_seconds(self, read_ext):
         if pd.read_excel.keywords["engine"] == "pyxlsb":
             pytest.xfail("Sheets containing datetimes not supported by pyxlsb")
@@ -1020,10 +1034,10 @@ def test_excel_read_buffer(self, engine, read_ext):
         tm.assert_frame_equal(expected, actual)
 
     def test_reader_closes_file(self, engine, read_ext):
-        f = open("test1" + read_ext, "rb")
-        with pd.ExcelFile(f) as xlsx:
-            # parses okay
-            pd.read_excel(xlsx, "Sheet1", index_col=0, engine=engine)
+        with open("test1" + read_ext, "rb") as f:
+            with pd.ExcelFile(f) as xlsx:
+                # parses okay
+                pd.read_excel(xlsx, "Sheet1", index_col=0, engine=engine)
 
         assert f.closed