fix test failures

Author: shoyer

doc/whats-new.rst

@@ -34,17 +34,16 @@ Bug fixes
 ~~~~~~~~~
 
 - Xarray objects opened from file-like objects with ``engine='h5netcdf'`` can
-  now be pickled, as long as the underlying file-like object also support
+  now be pickled, as long as the underlying file-like object also supports
   pickle
-  (:pull:`10624`).
+  (:issue:`10712`).
   By `Stephan Hoyer <https://github.com/shoyer>`_.
--  Closing Xarray objects opened from file-like objects with ```engine='scipy'``
+- Closing Xarray objects opened from file-like objects with ```engine='scipy'``
   no longer closes the underlying file, consistent the h5netcdf backend
   (:pull:`10624`).
   By `Stephan Hoyer <https://github.com/shoyer>`_.
 
 
-
 Documentation
 ~~~~~~~~~~~~~
 

xarray/backends/locks.py

@@ -100,7 +100,7 @@ def _get_threaded_lock(key: str) -> threading.Lock:
     return lock
 
 
-def _get_multiprocessing_lock(key: str) -> multiprocessing.Lock:
+def _get_multiprocessing_lock(key: str) -> Lock:
     # TODO: make use of the key -- maybe use locket.py?
     # https://github.com/mwilliamson/locket.py
     del key  # unused

xarray/backends/scipy_.py

@@ -35,12 +35,6 @@
 )
 from xarray.core.variable import Variable
 
-try:
-    from scipy.io import netcdf_file as netcdf_file_base
-except ImportError:
-    netcdf_file_base = object
-
-
 if TYPE_CHECKING:
     import scipy.io
 
@@ -110,30 +104,31 @@ def __setitem__(self, key, value):
                     raise
 
 
-class flush_only_netcdf_file(netcdf_file_base):
-    # scipy.io.netcdf_file.close() incorrectly closes file objects that
-    # were passed in as constructor arguments:
-    # https://github.com/scipy/scipy/issues/13905
-
-    # Instead of closing such files, only call flush(), which is
-    # equivalent as long as the netcdf_file object is not mmapped.
-    # This suffices to keep BytesIO objects open long enough to read
-    # their contents from to_netcdf(), but underlying files still get
-    # closed when the netcdf_file is garbage collected (via __del__),
-    # and will need to be fixed upstream in scipy.
-    def close(self):
-        if hasattr(self, "fp") and not self.fp.closed:
-            self.flush()
-
-    def __del__(self):
-        # Remove the __del__ method, which in scipy is aliased to close().
-        # These files need to be closed explicitly by xarray.
-        pass
-
-
 def _open_scipy_netcdf(filename, mode, mmap, version, flush_only=False):
     import scipy.io
 
+    # define inside a helper function to ensure the scipy import is lazy
+    class flush_only_netcdf_file(scipy.io.netcdf_file):
+        # scipy.io.netcdf_file.close() incorrectly closes file objects that
+        # were passed in as constructor arguments:
+        # https://github.com/scipy/scipy/issues/13905
+
+        # Instead of closing such files, only call flush(), which is
+        # equivalent as long as the netcdf_file object is not mmapped.
+        # This suffices to keep BytesIO objects open long enough to read
+        # their contents from to_netcdf(), but underlying files still get
+        # closed when the netcdf_file is garbage collected (via __del__),
+        # and will need to be fixed upstream in scipy.
+        def close(self):
+            if hasattr(self, "fp") and not self.fp.closed:
+                self.flush()
+                self.fp.seek(0)  # allow file to be read again
+
+        def __del__(self):
+            # Remove the __del__ method, which in scipy is aliased to close().
+            # These files need to be closed explicitly by xarray.
+            pass
+
     netcdf_file = flush_only_netcdf_file if flush_only else scipy.io.netcdf_file
 
     # if the string ends with .gz, then gunzip and open as netcdf file

xarray/tests/test_backends.py

@@ -116,12 +116,12 @@
 with contextlib.suppress(ImportError):
     import netCDF4 as nc4
 
-try:
+with contextlib.suppress(ImportError):
     import dask
     import dask.array as da
-except ImportError:
-    pass
 
+with contextlib.suppress(ImportError):
+    import fsspec
 
 if has_zarr:
     import zarr
@@ -633,16 +633,13 @@ def test_pickle(self) -> None:
             with pickle.loads(raw_pickle) as unpickled_ds:
                 assert_identical(expected, unpickled_ds)
 
-    @pytest.mark.filterwarnings("ignore:deallocating CachingFileManager")
     def test_pickle_dataarray(self) -> None:
         expected = Dataset({"foo": ("x", [42])})
         with self.roundtrip(expected, allow_cleanup_failure=ON_WINDOWS) as roundtripped:
             with roundtripped:
                 raw_pickle = pickle.dumps(roundtripped["foo"])
-            # TODO: figure out how to explicitly close the file for the
-            # unpickled DataArray?
-            unpickled = pickle.loads(raw_pickle)
-            assert_identical(expected["foo"], unpickled)
+            with pickle.loads(raw_pickle) as unpickled:
+                assert_identical(expected["foo"], unpickled)
 
     def test_dataset_caching(self) -> None:
         expected = Dataset({"foo": ("x", [5, 6, 7])})
@@ -658,7 +655,6 @@ def test_dataset_caching(self) -> None:
             _ = actual.foo.values  # no caching
             assert not actual.foo.variable._in_memory
 
-    @pytest.mark.filterwarnings("ignore:deallocating CachingFileManager")
     def test_roundtrip_None_variable(self) -> None:
         expected = Dataset({None: (("x", "y"), [[0, 1], [2, 3]])})
         with self.roundtrip(expected) as actual:
@@ -5113,7 +5109,6 @@ def test_open_badbytes(self) -> None:
 
     def test_open_twice(self) -> None:
         expected = create_test_data()
-        expected.attrs["foo"] = "bar"
         with create_tmp_file() as tmp_file:
             expected.to_netcdf(tmp_file, engine=self.engine)
             with open(tmp_file, "rb") as f:
@@ -5154,6 +5149,20 @@ def test_open_fileobj(self) -> None:
                 with open_dataset(f):  # ensure file gets closed
                     pass
 
+    @requires_fsspec
+    def test_fsspec(self) -> None:
+        expected = create_test_data()
+        with create_tmp_file() as tmp_file:
+            expected.to_netcdf(tmp_file, engine="h5netcdf")
+
+            with fsspec.open(tmp_file, "rb") as f:
+                with open_dataset(f, engine="h5netcdf") as actual:
+                    assert_identical(actual, expected)
+
+                    # fsspec.open() creates a pickleable file, unlike open()
+                    with pickle.loads(pickle.dumps(actual)) as unpickled:
+                        assert_identical(unpickled, expected)
+
 
 @requires_h5netcdf
 class TestH5NetCDFInMemoryData(InMemoryNetCDFWithGroups):