fix: working board_urls cache, which is flat

Author: machow

pins/boards.py

@@ -16,7 +16,7 @@
 from .errors import PinsError
 from .drivers import load_data, save_data, default_title
 from .utils import inform
-from .config import get_allow_rsc_short_name, get_feature_preview
+from .config import get_allow_rsc_short_name
 
 
 _log = logging.getLogger(__name__)
@@ -155,12 +155,10 @@ def pin_meta(self, name, version: str = None) -> Meta:
         meta_name = self.meta_factory.get_meta_name(*components)
 
         path_meta = self.construct_path([*components, meta_name])
-        f = self.fs.open(path_meta)
+        f = self._open_pin_meta(path_meta)
 
         meta = self.meta_factory.read_pin_yaml(f, pin_name, selected_version)
 
-        self._touch_cache(path_meta)
-
         return meta
 
     def pin_list(self):
@@ -213,11 +211,8 @@ def pin_read(self, name, version: Optional[str] = None, hash: Optional[str] = No
 
         pin_name = self.path_to_pin(name)
 
-        return load_data(
-            meta,
-            self.fs,
-            self.construct_path([pin_name, meta.version.version]),
-            allow_pickle_read=self.allow_pickle_read,
+        return self._load_data(
+            meta, self.construct_path([pin_name, meta.version.version])
         )
 
     def pin_write(
@@ -493,7 +488,13 @@ def pin_browse(self, name, version=None, local=False):
 
         raise NotImplementedError()
 
+    # pin name internal methods -----------------------------------------------
+    # these methods are responsible for validating pin names, and converting
+    # names to their ultimate path on the file system.
+
     def validate_pin_name(self, name: str) -> None:
+        """Raise an error if a pin name is not valid."""
+
         if "/" in name:
             raise ValueError(f"Invalid pin name: {name}")
 
@@ -512,6 +513,8 @@ def construct_path(self, elements) -> str:
     def keep_final_path_component(self, path):
         return path.split("/")[-1]
 
+    # version ordering, creation ----------------------------------------------
+
     def sort_pin_versions(self, versions):
         # assume filesystem returned them with most recent last
         return sorted(versions, key=lambda v: v.version)
@@ -574,10 +577,29 @@ def _extract_search_meta(self, meta):
         d["meta"] = meta
         return d
 
-    def _get_cache_path(self, pin_name, version):
-        p_version = self.construct_path([self.path_to_pin(pin_name), version])
-        hash = self.fs.hash_name(p_version, True)
-        return str(Path(self.fs.storage[-1]) / hash)
+    # data loading ------------------------------------------------------------
+
+    def _load_data(self, meta, pin_version_path):
+        """Return the data object stored by a pin (e.g. a DataFrame)."""
+        return load_data(
+            meta, self.fs, pin_version_path, allow_pickle_read=self.allow_pickle_read
+        )
+
+    # filesystem and cache methods --------------------------------------------
+
+    def _open_pin_meta(self, path):
+        f = self.fs.open(path)
+        self._touch_cache(path)
+
+        return f
+
+    def _get_cache_path(self, pin_name, version=None, fname=None):
+        version_part = [version] if version is not None else []
+        fname_part = [fname] if fname is not None else []
+        p_version = self.construct_path(
+            [self.path_to_pin(pin_name), *version_part, *fname_part]
+        )
+        return self.fs._check_file(p_version)
 
     def _touch_cache(self, path):
         from pins.cache import touch_access_time
@@ -587,8 +609,7 @@ def _touch_cache(self, path):
         if not hasattr(self.fs, "cached_files"):
             return
 
-        hash = self.fs.hash_name(path, True)
-        path_to_hashed = Path(self.fs.storage[-1]) / hash
+        path_to_hashed = self.fs._check_file(path)
         return touch_access_time(path_to_hashed)
 
 
@@ -611,9 +632,6 @@ class BoardManual(BaseBoard):
     0  1  a  3
     1  2  b  4
 
-
-
-
     """
 
     # TODO(question): is this class worth it? Or should the user just use fsspec?
@@ -623,14 +641,14 @@ def __init__(self, *args, pin_paths: dict, **kwargs):
 
         self.pin_paths = pin_paths
 
-    def pin_read(self, *args, **kwargs):
-        if not get_feature_preview():
-            raise NotImplementedError(
-                "pin_read with BoardManual is currently unimplemented. "
-                "See https://github.com/machow/pins-python/issues/59."
-            )
+    # def pin_read(self, *args, **kwargs):
+    #    if not get_feature_preview():
+    #        raise NotImplementedError(
+    #            "pin_read with BoardManual is currently unimplemented. "
+    #            "See https://github.com/machow/pins-python/issues/59."
+    #        )
 
-        return super().pin_read(*args, **kwargs)
+    #    return super().pin_read(*args, **kwargs)
 
     def pin_list(self):
         return list(self.pin_paths)
@@ -654,18 +672,20 @@ def pin_meta(self, name, version=None):
             return self.meta_factory.create_raw(path_to_pin, type="file", name=pin_name)
 
         path_meta = self.construct_path([pin_name, meta_name])
-        f = self.fs.open(path_meta)
+        f = self._open_pin_meta(path_meta)
         meta = self.meta_factory.read_pin_yaml(f, pin_name, VersionRaw(""))
 
+        # TODO(#59,#83): handle caching, and then re-enable pin_read.
+        # self._touch_cache(path_meta)
+
         return meta
 
     def pin_download(self, name, version=None, hash=None) -> Sequence[str]:
         meta = self.pin_meta(name, version)
 
         if isinstance(meta, MetaRaw):
-            return load_data(
-                meta, self.fs, None, allow_pickle_read=self.allow_pickle_read
-            )
+
+            return self._load_data(meta, None)
 
         raise NotImplementedError(
             "TODO: pin_download currently can only read a url to a single file."
@@ -686,6 +706,11 @@ def construct_path(self, elements):
             # boards forbid a final /, we need to strip it off to join elements
             pin_path = pin_path.rstrip().rstrip("/")
 
+            # this is a bit hacky, but this board only aims at specific pins versions,
+            # so the metadata has the version as "", so we need to remove it.
+            if others[0] == "":
+                return super().construct_path([pin_path, *others[1:]])
+
         return super().construct_path([pin_path, *others])
 
 

pins/cache.py

@@ -47,6 +47,16 @@ def protocol_to_string(protocol):
     return protocol[0]
 
 
+def prefix_cache(fs, board_base_path):
+    if isinstance(fs, str):
+        proto_name = fs
+    else:
+        proto_name = protocol_to_string(fs.protocol)
+    base_hash = hash_name(board_base_path, False)
+
+    return f"{proto_name}_{base_hash}"
+
+
 class PinsCache(SimpleCacheFileSystem):
     protocol = "pinscache"
 
@@ -154,6 +164,33 @@ def hash_name(self, path, same_name):
         return str(Path(full_prefix) / PLACEHOLDER_VERSION / final_part)
 
 
+class PinsAccessTimeCache(SimpleCacheFileSystem):
+    name = "pinsaccesstimecache"
+
+    def hash_name(self, path, same_name):
+        if same_name:
+            raise NotImplementedError("same_name not implemented.")
+
+        # hash full path, and put anything after the final / at the end, just
+        # to make it easier to browse.
+        base_name = super().hash_name(path, same_name)
+        suffix = Path(path).name
+        return f"{base_name}_{suffix}"
+
+    def _open(self, path, mode="rb", **kwargs):
+        f = super()._open(path, mode=mode, **kwargs)
+        fn = self._check_file(path)
+
+        if fn is None:
+            raise ValueError(
+                f"Cached file should exist for path, but none found: {path}"
+            )
+
+        touch_access_time(fn)
+
+        return f
+
+
 class CachePruner:
     """Prunes the cache directory, across multiple boards.
 

pins/constructors.py

@@ -3,7 +3,7 @@
 import tempfile
 
 from .boards import BaseBoard, BoardRsConnect, BoardManual
-from .cache import PinsCache, PinsUrlCache, PinsRscCache
+from .cache import PinsCache, PinsRscCache, PinsAccessTimeCache, prefix_cache
 from .config import get_data_dir, get_cache_dir
 
 
@@ -274,10 +274,10 @@ def board_urls(path: str, pin_paths: dict, cache=DEFAULT, allow_pickle_read=None
     Example
     -------
 
-    >>> github_raw = "https://raw.githubusercontent.com/"
+    >>> github_raw = "https://raw.githubusercontent.com/machow/pins-python/main/pins/tests/pins-compat"
     >>> pin_paths = {
-    ...     "df_csv": "df_csv/20220214T163720Z-9bfad",
-    ...     "df_arrow": "df_arrow/20220214T163720Z-ad0c1",
+    ...     "df_csv": "df_csv/20220214T163720Z-9bfad/",
+    ...     "df_arrow": "df_arrow/20220214T163720Z-ad0c1/",
     ... }
     >>> board = board_urls(github_raw, pin_paths)
     >>> board.pin_list()
@@ -289,8 +289,10 @@ def board_urls(path: str, pin_paths: dict, cache=DEFAULT, allow_pickle_read=None
         # copied from board(). this ensures that paths in cache have the form:
         # <full_path_hash>/<version_placeholder>/<file_name>
         cache_dir = get_cache_dir()
-        fs = PinsUrlCache(
-            target_protocol="http", cache_storage=cache_dir, same_names=True
+        sub_dir = prefix_cache("http", path)
+        sub_cache = f"{cache_dir}/{sub_dir}"
+        fs = PinsAccessTimeCache(
+            target_protocol="http", cache_storage=sub_cache, same_names=False
         )
     else:
         raise NotImplementedError("Can't currently pass own cache object")

pins/tests/test_constructors.py

@@ -65,12 +65,42 @@ def test_constructor_board_url_cache(tmp_cache, http_example_board_path, df_csv)
 
     assert len(http_dirs) == 1
 
+    # there are two files in the flat cache (metadata, and the csv)
     parent = http_dirs[0]
-    res = list(parent.rglob("**/*.csv"))
+    res = list(parent.rglob("*"))
+    assert len(res) == 2
+
+    # validate that it creates an empty metadata file
+    assert len(x for x in res if x.endswith("df_csv.csv")) == 1
+    assert len(x for x in res if x.endswith("data.txt")) == 1
+
+    assert len(list(parent.glob("**/*"))) == 2
+
+
+@pytest.mark.skip_on_github
+def test_constructor_board_url_file(tmp_cache, http_example_board_path):
+    # TODO: downloading a pin does not put files in the same directory, since
+    # in this case we are hashing on the full url.
+
+    board = c.board_urls(
+        http_example_board_path,
+        # could derive from example version path
+        pin_paths={"df_csv": "df_csv/20220214T163718Z-eceac/df_csv.csv"},
+    )
+
+    board.pin_download("df_csv")
+
+    # check cache ----
+    http_dirs = list(tmp_cache.glob("http_*"))
+
+    assert len(http_dirs) == 1
+
+    # there are two files in the flat cache (metadata, and the csv)
+    parent = http_dirs[0]
+    res = list(parent.rglob("*"))
     assert len(res) == 1
 
-    # has form: <pin>/<version>/<file>
-    check_cache_file_path(res[0], parent)
+    assert str(res[0]).endswith("df_csv.csv")
 
 
 @pytest.mark.skip_on_github