Merge pull request #293 from euroargodev/fix-upstream

Fixing upstream tests under Windows

argopy/stores/filesystems.py

@@ -216,17 +216,25 @@ def register(self, uri):
             if path not in self.cache_registry:
                 self.cache_registry.commit(path)
 
+    @property
+    def cached_files(self):
+        if version.parse(fsspec.__version__) <= version.parse("2023.6.0"):
+            return self.fs.cached_files
+        else:
+            # See https://github.com/euroargodev/argopy/issues/294
+            return self.fs._metadata.cached_files
+
     def cachepath(self, uri: str, errors: str = "raise"):
         """Return path to cached file for a given URI"""
         if not self.cache:
             if errors == "raise":
                 raise FileSystemHasNoCache("%s has no cache system" % type(self.fs))
         elif uri is not None:
             store_path = self.store_path(uri)
-            self.fs.load_cache()  # Read set of stored blocks from file and populate self.fs.cached_files
-            if store_path in self.fs.cached_files[-1]:
+            self.fs.load_cache()  # Read set of stored blocks from file and populate self.cached_files
+            if store_path in self.cached_files[-1]:
                 return os.path.sep.join(
-                    [self.cachedir, self.fs.cached_files[-1][store_path]["fn"]]
+                    [self.cachedir, self.cached_files[-1][store_path]["fn"]]
                 )
             elif errors == "raise":
                 raise CacheFileNotFound(
@@ -238,29 +246,40 @@ def cachepath(self, uri: str, errors: str = "raise"):
             )
 
     def _clear_cache_item(self, uri):
-        """Open fsspec cache registry (pickle file) and remove entry for uri"""
-        # See the "save_cache()" method in:
-        # https://filesystem-spec.readthedocs.io/en/latest/_modules/fsspec/implementations/cached.html#WholeFileCacheFileSystem
+        """Remove medadata and file for fsspec cache uri"""
         fn = os.path.join(self.fs.storage[-1], "cache")
-        self.fs.load_cache()  # Read set of stored blocks from file and populate self.fs.cached_files
-        cache = self.fs.cached_files[-1]
+        self.fs.load_cache()  # Read set of stored blocks from file and populate self.cached_files
+        cache = self.cached_files[-1]
+
+        # Read cache metadata:
         if os.path.exists(fn):
-            with open(fn, "rb") as f:
-                cached_files = pickle.load(
-                    f
-                )  # nosec B301 because files controlled internally
+            if version.parse(fsspec.__version__) <= version.parse("2023.6.0"):
+                with open(fn, "rb") as f:
+                    cached_files = pickle.load(f)  # nosec B301 because files controlled internally
+            else:
+                with open(fn, "r") as f:
+                    cached_files = json.load(f)
         else:
             cached_files = cache
+
+        # Build new metadata without uri to delete, and delete corresponding cached file:
         cache = {}
         for k, v in cached_files.items():
             if k != uri:
                 cache[k] = v.copy()
             else:
+                # Delete file:
                 os.remove(os.path.join(self.fs.storage[-1], v["fn"]))
                 # log.debug("Removed %s -> %s" % (uri, v['fn']))
-        with tempfile.NamedTemporaryFile(mode="wb", delete=False) as f:
-            pickle.dump(cache, f)
-        shutil.move(f.name, fn)
+
+        # Update cache metadata file:
+        if version.parse(fsspec.__version__) <= version.parse("2023.6.0"):
+            with tempfile.NamedTemporaryFile(mode="wb", delete=False) as f:
+                pickle.dump(cache, f)
+            shutil.move(f.name, fn)
+        else:
+            with fsspec.utils.atomic_write(fn, mode="w") as f:
+                json.dump(cache, f)
 
     def clear_cache(self):
         """Remove cache files and entry from uri open with this store instance"""

argopy/tests/helpers/mocked_http.py

@@ -8,9 +8,17 @@
 in "argopy/cli"
 
 Servers covered by this fixture:
-https://erddap.ifremer.fr/erddap
-https://github.com/euroargodev/argopy-data/raw/master
-https://api.ifremer.fr/argopy/data
+    "https://github.com/euroargodev/argopy-data/raw/master",
+    "https://erddap.ifremer.fr/erddap",
+    "https://data-argo.ifremer.fr",
+    "https://api.ifremer.fr",
+    "https://coastwatch.pfeg.noaa.gov/erddap",
+    "https://www.ocean-ops.org/api/1",
+    "https://dataselection.euro-argo.eu/api",
+    "https://vocab.nerc.ac.uk/collection",
+    "https://argovisbeta02.colorado.edu",
+    "https://dx.doi.org",
+    "https://archimer.ifremer.fr",
 
 The HTTPTestHandler class below is taken from the fsspec tests suite at:
 https://github.com/fsspec/filesystem_spec/blob/55c5d71e657445cbfbdba15049d660a5c9639ff0/fsspec/tests/conftest.py

argopy/tests/helpers/utils.py

@@ -20,6 +20,14 @@
 from packaging import version
 import warnings
 import logging
+import stat
+import platform
+import os
+from contextlib import contextmanager
+from enum import Enum
+from subprocess import check_output
+from pathlib import Path
+from typing import Generator, List
 
 from argopy.options import set_options
 from argopy.errors import ErddapServerError, ArgovisServerError, DataNotFound, FtpPathError
@@ -305,4 +313,66 @@ def test_wrapper(*args, **kwargs):
         fct_safe_to_server_errors(test_func)(*args, **kwargs)
     return test_wrapper
 
-log.debug("%s TESTS UTILS %s" % ("="*50, "="*50))
+
+def create_read_only_folder_linux(folder_path):
+    try:
+        # Create the folder
+        os.makedirs(folder_path, exist_ok=True)
+
+        # Get the current permissions of the folder
+        current_permissions = os.stat(folder_path).st_mode
+
+        # Remove the write access for the owner and group
+        new_permissions = current_permissions & ~(stat.S_IWUSR | stat.S_IWGRP)
+
+        # Set the new permissions
+        os.chmod(folder_path, new_permissions)
+
+    except FileExistsError:
+        log.debug(f"Folder '{folder_path}' already exists.")
+    except PermissionError:
+        log.debug("Error: You do not have sufficient permissions to create the folder.")
+
+
+def create_read_only_folder_windows(folder_path):
+    class AccessRight(Enum):
+        """Access Rights for files/folders"""
+
+        DELETE = "D"
+        FULL = "F"  # Edit Permissions + Create + Delete + Read + Write
+        NO_ACCESS = "N"
+        MODIFY = "M"  # Create + Delete + Read + Write
+        READ_EXECUTE = "RX"
+        READ_ONLY = "R"
+        WRITE_ONLY = "W"
+
+    def cmd(access_right: AccessRight, mode="grant:r") -> List[str]:
+        return [
+            "icacls",
+            str(folder_path),
+            "/inheritance:r",
+            f"/{mode}",
+            f"Everyone:{access_right.value}",
+        ]
+
+    try:
+        # Create the folder
+        os.makedirs(folder_path, exist_ok=True)
+
+        # Change permissions
+        warnings.warn(str(check_output(cmd(AccessRight.READ_ONLY))))
+
+    except FileExistsError:
+        log.debug(f"Folder '{folder_path}' already exists.")
+    except PermissionError:
+        log.debug("Error: You do not have sufficient permissions to create the folder.")
+
+
+def create_read_only_folder(folder_path):
+    if platform.system() == 'Windows':
+        create_read_only_folder_windows(folder_path)
+    else:
+        create_read_only_folder_linux(folder_path)
+
+
+log.debug("%s TESTS UTILS %s" % ("="*50, "="*50))

argopy/tests/test_options.py

@@ -1,9 +1,10 @@
 import os
 import pytest
+import platform
 import argopy
 from argopy.options import OPTIONS
 from argopy.errors import OptionValueError, FtpPathError, ErddapPathError
-from utils import requires_gdac
+from utils import requires_gdac, create_read_only_folder
 from mocked_http import mocked_httpserver, mocked_server_address
 import logging
 
@@ -52,30 +53,10 @@ def test_opt_dataset():
         assert OPTIONS["dataset"] == "ref"
 
 
-@pytest.mark.skipif(True, reason="Need to be debugged for Windows support")
+@pytest.mark.skipif(platform.system() == 'Windows', reason="Need to be debugged for Windows support")
 def test_opt_invalid_cachedir():
     # Cachedir is created if not exist.
     # OptionValueError is raised when it's not writable
-    import stat
-    def create_read_only_folder(folder_path):
-        try:
-            # Create the folder
-            os.makedirs(folder_path, exist_ok=True)
-
-            # Get the current permissions of the folder
-            current_permissions = os.stat(folder_path).st_mode
-
-            # Remove the write access for the owner and group
-            new_permissions = current_permissions & ~(stat.S_IWUSR | stat.S_IWGRP)
-
-            # Set the new permissions
-            os.chmod(folder_path, new_permissions)
-
-        except FileExistsError:
-            log.debug(f"Folder '{folder_path}' already exists.")
-        except PermissionError:
-            log.debug("Error: You do not have sufficient permissions to create the folder.")
-
     folder_name = "read_only_folder"
     create_read_only_folder(folder_name)
     with pytest.raises(OptionValueError):

argopy/tests/test_xarray_accessor.py

@@ -191,17 +191,17 @@ def is_valid_mdata(self, this_mdata):
         """Validate structure of the output dataset """
         check = []
         # Check for dimensions:
-        check.append(argopy.utilities.is_list_equal(['m', 'n'], list(this_mdata.dims)))
+        check.append(argopy.utils.is_list_equal(['m', 'n'], list(this_mdata.dims)))
         # Check for coordinates:
-        check.append(argopy.utilities.is_list_equal(['m', 'n'], list(this_mdata.coords)))
+        check.append(argopy.utils.is_list_equal(['m', 'n'], list(this_mdata.coords)))
         # Check for data variables:
         check.append(np.all(
             [v in this_mdata.data_vars for v in ['PRES', 'TEMP', 'PTMP', 'SAL', 'DATES', 'LAT', 'LONG', 'PROFILE_NO']]))
         check.append(np.all(
-            [argopy.utilities.is_list_equal(['n'], this_mdata[v].dims) for v in ['LONG', 'LAT', 'DATES', 'PROFILE_NO']
+            [argopy.utils.is_list_equal(['n'], this_mdata[v].dims) for v in ['LONG', 'LAT', 'DATES', 'PROFILE_NO']
              if v in this_mdata.data_vars]))
         check.append(np.all(
-            [argopy.utilities.is_list_equal(['m', 'n'], this_mdata[v].dims) for v in ['PRES', 'TEMP', 'SAL', 'PTMP'] if
+            [argopy.utils.is_list_equal(['m', 'n'], this_mdata[v].dims) for v in ['PRES', 'TEMP', 'SAL', 'PTMP'] if
              v in this_mdata.data_vars]))
         return np.all(check)
 

argopy/utils/caching.py

@@ -2,6 +2,7 @@
 import shutil
 import logging
 import pickle
+import json
 import fsspec
 import pandas as pd
 from packaging import version
@@ -62,14 +63,18 @@ def convert_size(size_bytes):
     cached_files = []
     fn = os.path.join(apath, "cache")
     if os.path.exists(fn):
-        with open(fn, "rb") as f:
-            loaded_cached_files = pickle.load(
-                f
-            )  # nosec B301 because files controlled internally
-            for c in loaded_cached_files.values():
-                if isinstance(c["blocks"], list):
-                    c["blocks"] = set(c["blocks"])
-            cached_files.append(loaded_cached_files)
+        if version.parse(fsspec.__version__) <= version.parse("2023.6.0"):
+            with open(fn, "rb") as f:
+                loaded_cached_files = pickle.load(
+                    f
+                )  # nosec B301 because files controlled internally
+        else:
+            with open(fn, "r") as f:
+                loaded_cached_files = json.load(f)
+        for c in loaded_cached_files.values():
+            if isinstance(c["blocks"], list):
+                c["blocks"] = set(c["blocks"])
+        cached_files.append(loaded_cached_files)
     else:
         raise FileSystemHasNoCache("No fsspec cache system at: %s" % apath)
 

argopy/utils/locals.py

@@ -168,7 +168,7 @@ def show_versions(file=sys.stdout, conda=False):  # noqa: C901
                 (
                     "pytest_reportlog",
                     lambda mod: mod.__version__,
-                ),  # will come with pandas
+                ),
                 ("setuptools", lambda mod: mod.__version__),
                 ("aiofiles", lambda mod: mod.__version__),
                 ("sphinx", lambda mod: mod.__version__),

ci/requirements/py3.8-all-free.yml

@@ -51,6 +51,7 @@ dependencies:
   - aiofiles
   - setuptools
 #  - sphinx
+  - requests
 
   - pip:
       - pytest-reportlog

ci/requirements/py3.9-all-free.yml

@@ -12,7 +12,7 @@ dependencies:
   - packaging
   - scipy
   - toolz
-  - xarray
+  - xarray  
 
 # EXT.UTIL:
   - gsw
@@ -51,6 +51,7 @@ dependencies:
   - aiofiles
   - setuptools
 #  - sphinx
+  - requests
 
   - pip:
       - pytest-reportlog

docs/whats-new.rst

@@ -17,6 +17,7 @@ Coming up next
 **Internals**
 
 - Utilities refactoring. All classes and functions have been refactored to more appropriate locations like ``argopy.utils`` or ``argopy.related``. A deprecation warning message will be displayed every time utilities are being used from the former locations. (:pr:`290`) by `G. Maze <http://www.github.com/gmaze>`_
+- Fix bugs due to fsspec new internal cache handling and Windows specifics. (:pr:`293`) by `G. Maze <http://www.github.com/gmaze>`_
 
 v0.1.14rc2 (27 Jul. 2023)
 -------------------------