Merge branch 'main' into indexes-html-repr

Author: keewis

doc/whats-new.rst

@@ -45,6 +45,10 @@ Deprecations
 Bug fixes
 ~~~~~~~~~
 
+- Explicitly opening a file multiple times (e.g., after modifying it on disk)
+  now reopens the file from scratch for h5netcdf and scipy netCDF backends,
+  rather than reusing a cached version (:issue:`4240`, :issue:`4862`).
+  By `Stephan Hoyer <https://github.com/shoyer>`_.
 
 Documentation
 ~~~~~~~~~~~~~
@@ -59,8 +63,11 @@ Internal Changes
 ~~~~~~~~~~~~~~~~
 - Doctests fail on any warnings (:pull:`7166`)
   By `Maximilian Roos <https://github.com/max-sixty>`_.
-
-
+- Improve import time by lazy loading ``dask.distributed`` (:pull: `7172`).
+- Explicitly specify ``longdouble=False`` in :py:func:`cftime.date2num` when
+  encoding times to preserve existing behavior and prevent future errors when it
+  is eventually set to ``True`` by default in cftime (:pull:`7171`).  By
+  `Spencer Clark <https://github.com/spencerkclark>`_.
 
 .. _whats-new.2022.10.0:
 

xarray/backends/file_manager.py

@@ -3,21 +3,21 @@
 import contextlib
 import io
 import threading
+import uuid
 import warnings
-from typing import Any
+from typing import Any, Hashable
 
 from ..core import utils
 from ..core.options import OPTIONS
 from .locks import acquire
 from .lru_cache import LRUCache
 
 # Global cache for storing open files.
-FILE_CACHE: LRUCache[str, io.IOBase] = LRUCache(
+FILE_CACHE: LRUCache[Any, io.IOBase] = LRUCache(
     maxsize=OPTIONS["file_cache_maxsize"], on_evict=lambda k, v: v.close()
 )
 assert FILE_CACHE.maxsize, "file cache must be at least size one"
 
-
 REF_COUNTS: dict[Any, int] = {}
 
 _DEFAULT_MODE = utils.ReprObject("<unused>")
@@ -85,12 +85,13 @@ def __init__(
         kwargs=None,
         lock=None,
         cache=None,
+        manager_id: Hashable | None = None,
         ref_counts=None,
     ):
-        """Initialize a FileManager.
+        """Initialize a CachingFileManager.
 
-        The cache and ref_counts arguments exist solely to facilitate
-        dependency injection, and should only be set for tests.
+        The cache, manager_id and ref_counts arguments exist solely to
+        facilitate dependency injection, and should only be set for tests.
 
         Parameters
         ----------
@@ -120,6 +121,8 @@ def __init__(
             global variable and contains non-picklable file objects, an
             unpickled FileManager objects will be restored with the default
             cache.
+        manager_id : hashable, optional
+            Identifier for this CachingFileManager.
         ref_counts : dict, optional
             Optional dict to use for keeping track the number of references to
             the same file.
@@ -129,13 +132,17 @@ def __init__(
         self._mode = mode
         self._kwargs = {} if kwargs is None else dict(kwargs)
 
-        self._default_lock = lock is None or lock is False
-        self._lock = threading.Lock() if self._default_lock else lock
+        self._use_default_lock = lock is None or lock is False
+        self._lock = threading.Lock() if self._use_default_lock else lock
 
         # cache[self._key] stores the file associated with this object.
         if cache is None:
             cache = FILE_CACHE
         self._cache = cache
+        if manager_id is None:
+            # Each call to CachingFileManager should separately open files.
+            manager_id = str(uuid.uuid4())
+        self._manager_id = manager_id
         self._key = self._make_key()
 
         # ref_counts[self._key] stores the number of CachingFileManager objects
@@ -153,6 +160,7 @@ def _make_key(self):
             self._args,
             "a" if self._mode == "w" else self._mode,
             tuple(sorted(self._kwargs.items())),
+            self._manager_id,
         )
         return _HashedSequence(value)
 
@@ -223,20 +231,14 @@ def close(self, needs_lock=True):
             if file is not None:
                 file.close()
 
-    def __del__(self):
-        # If we're the only CachingFileManger referencing a unclosed file, we
-        # should remove it from the cache upon garbage collection.
+    def __del__(self) -> None:
+        # If we're the only CachingFileManger referencing a unclosed file,
+        # remove it from the cache upon garbage collection.
         #
-        # Keeping our own count of file references might seem like overkill,
-        # but it's actually pretty common to reopen files with the same
-        # variable name in a notebook or command line environment, e.g., to
-        # fix the parameters used when opening a file:
-        #    >>> ds = xarray.open_dataset('myfile.nc')
-        #    >>> ds = xarray.open_dataset('myfile.nc', decode_times=False)
-        # This second assignment to "ds" drops CPython's ref-count on the first
-        # "ds" argument to zero, which can trigger garbage collections. So if
-        # we didn't check whether another object is referencing 'myfile.nc',
-        # the newly opened file would actually be immediately closed!
+        # We keep track of our own reference count because we don't want to
+        # close files if another identical file manager needs it. This can
+        # happen if a CachingFileManager is pickled and unpickled without
+        # closing the original file.
         ref_count = self._ref_counter.decrement(self._key)
 
         if not ref_count and self._key in self._cache:
@@ -249,30 +251,40 @@ def __del__(self):
 
             if OPTIONS["warn_for_unclosed_files"]:
                 warnings.warn(
-                    "deallocating {}, but file is not already closed. "
-                    "This may indicate a bug.".format(self),
+                    f"deallocating {self}, but file is not already closed. "
+                    "This may indicate a bug.",
                     RuntimeWarning,
                     stacklevel=2,
                 )
 
     def __getstate__(self):
         """State for pickling."""
-        # cache and ref_counts are intentionally omitted: we don't want to try
-        # to serialize these global objects.
-        lock = None if self._default_lock else self._lock
-        return (self._opener, self._args, self._mode, self._kwargs, lock)
+        # cache is intentionally omitted: we don't want to try to serialize
+        # these global objects.
+        lock = None if self._use_default_lock else self._lock
+        return (
+            self._opener,
+            self._args,
+            self._mode,
+            self._kwargs,
+            lock,
+            self._manager_id,
+        )
 
-    def __setstate__(self, state):
+    def __setstate__(self, state) -> None:
         """Restore from a pickle."""
-        opener, args, mode, kwargs, lock = state
-        self.__init__(opener, *args, mode=mode, kwargs=kwargs, lock=lock)
+        opener, args, mode, kwargs, lock, manager_id = state
+        self.__init__(  # type: ignore
+            opener, *args, mode=mode, kwargs=kwargs, lock=lock, manager_id=manager_id
+        )
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         args_string = ", ".join(map(repr, self._args))
         if self._mode is not _DEFAULT_MODE:
             args_string += f", mode={self._mode!r}"
-        return "{}({!r}, {}, kwargs={})".format(
-            type(self).__name__, self._opener, args_string, self._kwargs
+        return (
+            f"{type(self).__name__}({self._opener!r}, {args_string}, "
+            f"kwargs={self._kwargs}, manager_id={self._manager_id!r})"
         )
 
 

xarray/backends/locks.py

@@ -11,11 +11,6 @@
     # no need to worry about serializing the lock
     SerializableLock = threading.Lock  # type: ignore
 
-try:
-    from dask.distributed import Lock as DistributedLock
-except ImportError:
-    DistributedLock = None  # type: ignore
-
 
 # Locks used by multiple backends.
 # Neither HDF5 nor the netCDF-C library are thread-safe.
@@ -41,14 +36,6 @@ def _get_multiprocessing_lock(key):
     return multiprocessing.Lock()
 
 
-_LOCK_MAKERS = {
-    None: _get_threaded_lock,
-    "threaded": _get_threaded_lock,
-    "multiprocessing": _get_multiprocessing_lock,
-    "distributed": DistributedLock,
-}
-
-
 def _get_lock_maker(scheduler=None):
     """Returns an appropriate function for creating resource locks.
 
@@ -61,7 +48,23 @@ def _get_lock_maker(scheduler=None):
     --------
     dask.utils.get_scheduler_lock
     """
-    return _LOCK_MAKERS[scheduler]
+
+    if scheduler is None:
+        return _get_threaded_lock
+    elif scheduler == "threaded":
+        return _get_threaded_lock
+    elif scheduler == "multiprocessing":
+        return _get_multiprocessing_lock
+    elif scheduler == "distributed":
+        # Lazy import distributed since it is can add a significant
+        # amount of time to import
+        try:
+            from dask.distributed import Lock as DistributedLock
+        except ImportError:
+            DistributedLock = None  # type: ignore
+        return DistributedLock
+    else:
+        raise KeyError(scheduler)
 
 
 def _get_scheduler(get=None, collection=None) -> str | None:
@@ -128,15 +131,12 @@ def acquire(lock, blocking=True):
     if blocking:
         # no arguments needed
         return lock.acquire()
-    elif DistributedLock is not None and isinstance(lock, DistributedLock):
-        # distributed.Lock doesn't support the blocking argument yet:
-        # https://github.com/dask/distributed/pull/2412
-        return lock.acquire(timeout=0)
     else:
         # "blocking" keyword argument not supported for:
         # - threading.Lock on Python 2.
         # - dask.SerializableLock with dask v1.0.0 or earlier.
         # - multiprocessing.Lock calls the argument "block" instead.
+        # - dask.distributed.Lock uses the blocking argument as the first one
         return lock.acquire(blocking)
 
 

xarray/coding/times.py

@@ -581,7 +581,18 @@ def _encode_datetime_with_cftime(dates, units, calendar):
         dates = dates.astype("M8[us]").astype(datetime)
 
     def encode_datetime(d):
-        return np.nan if d is None else cftime.date2num(d, units, calendar)
+        # Since netCDF files do not support storing float128 values, we ensure
+        # that float64 values are used by setting longdouble=False in num2date.
+        # This try except logic can be removed when xarray's minimum version of
+        # cftime is at least 1.6.2.
+        try:
+            return (
+                np.nan
+                if d is None
+                else cftime.date2num(d, units, calendar, longdouble=False)
+            )
+        except TypeError:
+            return np.nan if d is None else cftime.date2num(d, units, calendar)
 
     return np.array([encode_datetime(d) for d in dates.ravel()]).reshape(dates.shape)
 

xarray/conventions.py

@@ -141,7 +141,7 @@ def maybe_encode_bools(var):
     ):
         dims, data, attrs, encoding = _var_as_tuple(var)
         attrs["dtype"] = "bool"
-        data = data.astype(dtype="i1", copy=True)
+        data = duck_array_ops.astype(data, dtype="i1", copy=True)
         var = Variable(dims, data, attrs, encoding)
     return var