Backport PR #51443 on branch 2.0.x (CI/TST: Error on PytestUnraisableExceptionWarning instead of using psutil to check open resources) (#51507)

meeseeksmachine · mroeschke · phofl · web-flow · commit a3894fde6b67 · 2023-02-21T14:03:53.000Z
Backport PR #51443: CI/TST: Error on PytestUnraisableExceptionWarning instead of using psutil to check open resources Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> Co-authored-by: Patrick Hoefler <61934744+phofl@users.noreply.github.com>
diff --git a/ci/deps/actions-310.yaml b/ci/deps/actions-310.yaml
@@ -12,7 +12,6 @@ dependencies:
   - pytest>=7.0.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - boto3
 
diff --git a/ci/deps/actions-311.yaml b/ci/deps/actions-311.yaml
@@ -12,7 +12,6 @@ dependencies:
   - pytest>=7.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - boto3
 
diff --git a/ci/deps/actions-38-downstream_compat.yaml b/ci/deps/actions-38-downstream_compat.yaml
@@ -13,7 +13,6 @@ dependencies:
   - pytest>=7.0.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - boto3
 
diff --git a/ci/deps/actions-38-minimum_versions.yaml b/ci/deps/actions-38-minimum_versions.yaml
@@ -14,7 +14,6 @@ dependencies:
   - pytest>=7.0.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - boto3
 
diff --git a/ci/deps/actions-38.yaml b/ci/deps/actions-38.yaml
@@ -12,7 +12,6 @@ dependencies:
   - pytest>=7.0.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - boto3
 
diff --git a/ci/deps/actions-39.yaml b/ci/deps/actions-39.yaml
@@ -12,7 +12,6 @@ dependencies:
   - pytest>=7.0.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - boto3
 
diff --git a/ci/deps/circle-38-arm64.yaml b/ci/deps/circle-38-arm64.yaml
@@ -12,7 +12,6 @@ dependencies:
   - pytest>=7.0.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - boto3
 
diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst
@@ -1328,6 +1328,7 @@ I/O
 - Bug in :meth:`DataFrame.to_html` with ``na_rep`` set when the :class:`DataFrame` contains non-scalar data (:issue:`47103`)
 - Bug in :func:`read_xml` where file-like objects failed when iterparse is used (:issue:`50641`)
 - Bug in :func:`read_xml` ignored repeated elements when iterparse is used (:issue:`51183`)
+- Bug in :class:`ExcelWriter` leaving file handles open if an exception occurred during instantiation (:issue:`51443`)
 
 Period
 ^^^^^^
diff --git a/environment.yml b/environment.yml
@@ -14,7 +14,6 @@ dependencies:
   - pytest>=7.0.0
   - pytest-cov
   - pytest-xdist>=2.2.0
-  - psutil
   - pytest-asyncio>=0.17
   - coverage
 
diff --git a/pandas/_testing/_warnings.py b/pandas/_testing/_warnings.py
@@ -5,7 +5,6 @@
     nullcontext,
 )
 import re
-import sys
 from typing import (
     Generator,
     Literal,
@@ -164,22 +163,6 @@ def _assert_caught_no_extra_warnings(
 
     for actual_warning in caught_warnings:
         if _is_unexpected_warning(actual_warning, expected_warning):
-            # GH#38630 pytest.filterwarnings does not suppress these.
-            if actual_warning.category == ResourceWarning:
-                # GH 44732: Don't make the CI flaky by filtering SSL-related
-                # ResourceWarning from dependencies
-                unclosed_ssl = (
-                    "unclosed transport <asyncio.sslproto._SSLProtocolTransport",
-                    "unclosed <ssl.SSLSocket",
-                )
-                if any(msg in str(actual_warning.message) for msg in unclosed_ssl):
-                    continue
-                # GH 44844: Matplotlib leaves font files open during the entire process
-                # upon import. Don't make CI flaky if ResourceWarning raised
-                # due to these open files.
-                if any("matplotlib" in mod for mod in sys.modules):
-                    continue
-
             extra_warnings.append(
                 (
                     actual_warning.category.__name__,
diff --git a/pandas/io/excel/_base.py b/pandas/io/excel/_base.py
@@ -1210,6 +1210,17 @@ def __init__(
         # the excel backend first read the existing file and then write any data to it
         mode = mode.replace("a", "r+")
 
+        if if_sheet_exists not in (None, "error", "new", "replace", "overlay"):
+            raise ValueError(
+                f"'{if_sheet_exists}' is not valid for if_sheet_exists. "
+                "Valid options are 'error', 'new', 'replace' and 'overlay'."
+            )
+        if if_sheet_exists and "r+" not in mode:
+            raise ValueError("if_sheet_exists is only valid in append mode (mode='a')")
+        if if_sheet_exists is None:
+            if_sheet_exists = "error"
+        self._if_sheet_exists = if_sheet_exists
+
         # cast ExcelWriter to avoid adding 'if self._handles is not None'
         self._handles = IOHandles(
             cast(IO[bytes], path), compression={"compression": None}
@@ -1231,17 +1242,6 @@ def __init__(
 
         self._mode = mode
 
-        if if_sheet_exists not in (None, "error", "new", "replace", "overlay"):
-            raise ValueError(
-                f"'{if_sheet_exists}' is not valid for if_sheet_exists. "
-                "Valid options are 'error', 'new', 'replace' and 'overlay'."
-            )
-        if if_sheet_exists and "r+" not in mode:
-            raise ValueError("if_sheet_exists is only valid in append mode (mode='a')")
-        if if_sheet_exists is None:
-            if_sheet_exists = "error"
-        self._if_sheet_exists = if_sheet_exists
-
     @property
     def date_format(self) -> str:
         """
@@ -1602,11 +1602,3 @@ def __exit__(
         traceback: TracebackType | None,
     ) -> None:
         self.close()
-
-    def __del__(self) -> None:
-        # Ensure we don't leak file descriptors, but put in try/except in case
-        # attributes are already deleted
-        try:
-            self.close()
-        except AttributeError:
-            pass
diff --git a/pandas/io/excel/_odswriter.py b/pandas/io/excel/_odswriter.py
@@ -48,6 +48,9 @@ def __init__(
         if mode == "a":
             raise ValueError("Append mode is not supported with odf!")
 
+        engine_kwargs = combine_kwargs(engine_kwargs, kwargs)
+        self._book = OpenDocumentSpreadsheet(**engine_kwargs)
+
         super().__init__(
             path,
             mode=mode,
@@ -56,9 +59,6 @@ def __init__(
             engine_kwargs=engine_kwargs,
         )
 
-        engine_kwargs = combine_kwargs(engine_kwargs, kwargs)
-
-        self._book = OpenDocumentSpreadsheet(**engine_kwargs)
         self._style_dict: dict[str, str] = {}
 
     @property
diff --git a/pandas/io/excel/_openpyxl.py b/pandas/io/excel/_openpyxl.py
@@ -70,11 +70,19 @@ def __init__(
         if "r+" in self._mode:  # Load from existing workbook
             from openpyxl import load_workbook
 
-            self._book = load_workbook(self._handles.handle, **engine_kwargs)
+            try:
+                self._book = load_workbook(self._handles.handle, **engine_kwargs)
+            except TypeError:
+                self._handles.handle.close()
+                raise
             self._handles.handle.seek(0)
         else:
             # Create workbook object with default optimized_write=True.
-            self._book = Workbook(**engine_kwargs)
+            try:
+                self._book = Workbook(**engine_kwargs)
+            except TypeError:
+                self._handles.handle.close()
+                raise
 
             if self.book.worksheets:
                 self.book.remove(self.book.worksheets[0])
diff --git a/pandas/tests/frame/test_api.py b/pandas/tests/frame/test_api.py
@@ -7,7 +7,6 @@
 
 from pandas._config.config import option_context
 
-import pandas.util._test_decorators as td
 from pandas.util._test_decorators import (
     async_mark,
     skip_if_no,
@@ -293,7 +292,6 @@ def _check_f(base, f):
         _check_f(d.copy(), f)
 
     @async_mark()
-    @td.check_file_leaks
     async def test_tab_complete_warning(self, ip, frame_or_series):
         # GH 16409
         pytest.importorskip("IPython", minversion="6.0.0")
diff --git a/pandas/tests/io/excel/conftest.py b/pandas/tests/io/excel/conftest.py
@@ -1,8 +1,5 @@
 import pytest
 
-from pandas.compat import is_platform_windows
-import pandas.util._test_decorators as td
-
 import pandas._testing as tm
 
 from pandas.io.parsers import read_csv
@@ -42,26 +39,3 @@ def read_ext(request):
     Valid extensions for reading Excel files.
     """
     return request.param
-
-
-# Checking for file leaks can hang on Windows CI
-@pytest.fixture(autouse=not is_platform_windows())
-def check_for_file_leaks():
-    """
-    Fixture to run around every test to ensure that we are not leaking files.
-
-    See also
-    --------
-    _test_decorators.check_file_leaks
-    """
-    # GH#30162
-    psutil = td.safe_import("psutil")
-    if not psutil:
-        yield
-
-    else:
-        proc = psutil.Process()
-        flist = proc.open_files()
-        yield
-        flist2 = proc.open_files()
-        assert flist == flist2
diff --git a/pandas/tests/io/excel/test_readers.py b/pandas/tests/io/excel/test_readers.py
@@ -909,7 +909,6 @@ def test_read_from_pathlib_path(self, read_ext):
         tm.assert_frame_equal(expected, actual)
 
     @td.skip_if_no("py.path")
-    @td.check_file_leaks
     def test_read_from_py_localpath(self, read_ext):
         # GH12655
         from py.path import local as LocalPath
@@ -922,7 +921,6 @@ def test_read_from_py_localpath(self, read_ext):
 
         tm.assert_frame_equal(expected, actual)
 
-    @td.check_file_leaks
     def test_close_from_py_localpath(self, read_ext):
         # GH31467
         str_path = os.path.join("test1" + read_ext)
diff --git a/pandas/tests/io/formats/test_format.py b/pandas/tests/io/formats/test_format.py
@@ -26,7 +26,6 @@
     IS64,
     is_platform_windows,
 )
-import pandas.util._test_decorators as td
 
 import pandas as pd
 from pandas import (
@@ -3400,7 +3399,6 @@ def test_format_percentiles_integer_idx():
     assert result == expected
 
 
-@td.check_file_leaks
 def test_repr_html_ipython_config(ip):
     code = textwrap.dedent(
         """\
diff --git a/pandas/tests/io/parser/common/test_file_buffer_url.py b/pandas/tests/io/parser/common/test_file_buffer_url.py
@@ -13,7 +13,6 @@
 
 import pytest
 
-from pandas.compat import is_ci_environment
 from pandas.errors import (
     EmptyDataError,
     ParserError,
@@ -404,25 +403,14 @@ def test_context_manageri_user_provided(all_parsers, datapath):
             assert not reader.handles.handle.closed
 
 
-def test_file_descriptor_leak(all_parsers, using_copy_on_write, request):
+def test_file_descriptor_leak(all_parsers, using_copy_on_write):
     # GH 31488
-    if using_copy_on_write and is_ci_environment():
-        mark = pytest.mark.xfail(
-            reason="2023-02-12 frequent-but-flaky failures", strict=False
-        )
-        request.node.add_marker(mark)
-
     parser = all_parsers
     with tm.ensure_clean() as path:
-
-        def test():
-            with pytest.raises(EmptyDataError, match="No columns to parse from file"):
-                parser.read_csv(path)
-
-        td.check_file_leaks(test)()
+        with pytest.raises(EmptyDataError, match="No columns to parse from file"):
+            parser.read_csv(path)
 
 
-@td.check_file_leaks
 def test_memory_map(all_parsers, csv_dir_path):
     mmap_file = os.path.join(csv_dir_path, "test_mmap.csv")
     parser = all_parsers
diff --git a/pandas/tests/io/parser/common/test_read_errors.py b/pandas/tests/io/parser/common/test_read_errors.py
@@ -17,7 +17,6 @@
     EmptyDataError,
     ParserError,
 )
-import pandas.util._test_decorators as td
 
 from pandas import DataFrame
 import pandas._testing as tm
@@ -204,7 +203,6 @@ def test_null_byte_char(request, all_parsers):
             parser.read_csv(StringIO(data), names=names)
 
 
-@td.check_file_leaks
 def test_open_file(request, all_parsers):
     # GH 39024
     parser = all_parsers
diff --git a/pandas/tests/io/sas/test_xport.py b/pandas/tests/io/sas/test_xport.py
@@ -1,8 +1,6 @@
 import numpy as np
 import pytest
 
-import pandas.util._test_decorators as td
-
 import pandas as pd
 import pandas._testing as tm
 
@@ -21,11 +19,6 @@ def numeric_as_float(data):
 
 
 class TestXport:
-    @pytest.fixture(autouse=True)
-    def setup_method(self):
-        with td.file_leak_context():
-            yield
-
     @pytest.fixture
     def file01(self, datapath):
         return datapath("io", "sas", "data", "DEMO_G.xpt")
@@ -138,10 +131,9 @@ def test2_binary(self, file02):
         numeric_as_float(data_csv)
 
         with open(file02, "rb") as fd:
-            with td.file_leak_context():
-                # GH#35693 ensure that if we pass an open file, we
-                #  dont incorrectly close it in read_sas
-                data = read_sas(fd, format="xport")
+            # GH#35693 ensure that if we pass an open file, we
+            #  dont incorrectly close it in read_sas
+            data = read_sas(fd, format="xport")
 
         tm.assert_frame_equal(data, data_csv)
 
diff --git a/pandas/tests/resample/test_resampler_grouper.py b/pandas/tests/resample/test_resampler_grouper.py
@@ -3,7 +3,6 @@
 import numpy as np
 import pytest
 
-import pandas.util._test_decorators as td
 from pandas.util._test_decorators import async_mark
 
 import pandas as pd
@@ -24,7 +23,6 @@
 
 
 @async_mark()
-@td.check_file_leaks
 async def test_tab_complete_ipython6_warning(ip):
     from IPython.core.completer import provisionalcompleter
 
diff --git a/pandas/tests/scalar/timestamp/test_timestamp.py b/pandas/tests/scalar/timestamp/test_timestamp.py
diff --git a/pandas/util/_test_decorators.py b/pandas/util/_test_decorators.py
diff --git a/pyproject.toml b/pyproject.toml
diff --git a/requirements-dev.txt b/requirements-dev.txt
