fix: correct cache paths

Author: machow

pins/boards.py

@@ -45,9 +45,13 @@ def info(self, path):
 
 class BaseBoard:
     def __init__(
-        self, board: str, fs: IFileSystem, versioned=True, meta_factory=MetaFactory(),
+        self,
+        board: "str | Path",
+        fs: IFileSystem,
+        versioned=True,
+        meta_factory=MetaFactory(),
     ):
-        self.board = board
+        self.board = str(board)
         self.fs = fs
         self.meta_factory = meta_factory
 

pins/cache.py

@@ -36,6 +36,13 @@ def touch_access_time(path, access_time: "float | None" = None):
     return access_time
 
 
+def protocol_to_string(protocol):
+    if isinstance(protocol, str):
+        return protocol
+
+    return protocol[0]
+
+
 class PinsCache(SimpleCacheFileSystem):
     protocol = "pinscache"
 
@@ -74,7 +81,8 @@ def hash_name(self, path, same_name):
 
                 # TODO: hacky to automatically tack on protocol here
                 # but this is what R pins boards do. Could make a bool arg?
-                full_prefix = "_".join([self.fs.protocol, prefix])
+                proto_name = protocol_to_string(self.fs.protocol)
+                full_prefix = "_".join([proto_name, prefix])
                 return str(full_prefix / suffix)
 
             return path
@@ -109,7 +117,9 @@ def hash_name(self, path, same_name):
 
         # note that we include an extra version folder, so it conforms with
         # pin board path form: <board_path>/<pin_name>/<version_name>/<file>
-        return str(Path(prefix) / PLACEHOLDER_VERSION / final_part)
+        proto_name = protocol_to_string(self.fs.protocol)
+        full_prefix = "_".join([proto_name, prefix])
+        return str(Path(full_prefix) / PLACEHOLDER_VERSION / final_part)
 
 
 class CachePruner:

pins/tests/conftest.py

@@ -7,7 +7,10 @@
 from pathlib import Path
 from pins.tests.helpers import BoardBuilder, RscBoardBuilder, Snapshot, rm_env
 
+EXAMPLE_REL_PATH = "pins/tests/pins-compat"
 PATH_TO_EXAMPLE_BOARD = files("pins") / "tests/pins-compat"
+PATH_TO_EXAMPLE_VERSION = PATH_TO_EXAMPLE_BOARD / "df_csv/20220214T163720Z-9bfad/"
+EXAMPLE_PIN_NAME = "df_csv"
 
 
 # Based on https://github.com/machow/siuba/blob/main/siuba/tests/helpers.py
@@ -66,19 +69,19 @@ def tmp_dir2():
 
 
 @pytest.fixture
-def tmp_cache(tmp_dir2):
+def tmp_cache():
     with rm_env("PINS_CACHE_DIR"):
-        os.environ["PINS_CACHE_DIR"] = str(tmp_dir2)
-
-        yield tmp_dir2
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            os.environ["PINS_CACHE_DIR"] = str(tmp_dir)
+            yield Path(tmp_dir)
 
 
 @pytest.fixture
-def tmp_data_dir(tmp_dir2):
+def tmp_data_dir():
     with rm_env("PINS_DATA_DIR"):
-        os.environ["PINS_DATA_DIR"] = str(tmp_dir2)
-
-        yield tmp_dir2
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            os.environ["PINS_DATA_DIR"] = str(tmp_dir)
+            yield Path(tmp_dir)
 
 
 def pytest_addoption(parser):

pins/tests/test_constructors.py

@@ -1,38 +1,90 @@
 import os
+import pandas as pd
 import pytest
 
+from pandas.testing import assert_frame_equal
 from pathlib import Path
 
 from pins import constructors as c
-from pins.tests.conftest import PATH_TO_EXAMPLE_BOARD
+from pins.tests.conftest import (
+    PATH_TO_EXAMPLE_BOARD,
+    PATH_TO_EXAMPLE_VERSION,
+    EXAMPLE_REL_PATH,
+)
 from pins.tests.helpers import rm_env
 
 
-# adapted from https://stackoverflow.com/a/34333710
+@pytest.fixture
+def df_csv():
+    return pd.read_csv(PATH_TO_EXAMPLE_VERSION / "df_csv.csv", index_col=0)
 
 
 def check_dir_writable(p_dir):
     assert p_dir.parent.exists()
     assert os.access(p_dir.parent.absolute(), os.W_OK)
 
 
+def check_cache_file_path(p_file, p_cache):
+    assert str(p_file.relative_to(p_cache)).count("/") == 2
+
+
 # End-to-end constructor tests
 
 # there are two facets of boards: reading and writing.
 # copied from test_compat
-def test_constructor_board_url(tmp_cache, http_example_board_path):
+def test_constructor_board_url_data(tmp_cache, http_example_board_path, df_csv):
+    board = c.board_urls(
+        http_example_board_path,
+        # could derive from example version path
+        pin_paths={"df_csv": "df_csv/20220214T163720Z-9bfad/"},
+    )
+
+    df = board.pin_read("df_csv")
+
+    # check data ----
+    assert_frame_equal(df, df_csv)
+
+
+@pytest.mark.xfail
+def test_constructor_board_url_cache(tmp_cache, http_example_board_path, df_csv):
+    # 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, pin_paths={"df_csv": "df_csv/20220214T163718Z-eceac/"}
+        http_example_board_path,
+        # could derive from example version path
+        pin_paths={"df_csv": "df_csv/20220214T163718Z-eceac/"},
     )
 
     board.pin_read("df_csv")
 
-    # check cache
-    # check data
+    # check cache ----
+    http_dirs = list(tmp_cache.glob("http_*"))
+
+    assert len(http_dirs) == 1
 
+    parent = http_dirs[0]
+    res = list(parent.rglob("**/*.csv"))
+    assert len(res) == 1
 
-def test_constructor_board_github(tmp_cache, http_example_board_path):
-    board = c.board_github("machow", "pins-python", PATH_TO_EXAMPLE_BOARD)  # noqa
+    # has form: <pin>/<version>/<file>
+    check_cache_file_path(res[0], parent)
+
+
+def test_constructor_board_github(tmp_cache, http_example_board_path, df_csv):
+    board = c.board_github("machow", "pins-python", EXAMPLE_REL_PATH)  # noqa
+
+    df = board.pin_read("df_csv")
+    assert_frame_equal(df, df_csv)
+
+    cache_options = list(tmp_cache.glob("github_*"))
+    assert len(cache_options) == 1
+    cache_dir = cache_options[0]
+
+    res = list(cache_dir.rglob("**/*.csv"))
+    assert len(res) == 1
+
+    check_cache_file_path(res[0], cache_dir)
 
 
 @pytest.fixture(scope="session")
@@ -44,7 +96,7 @@ def board(backend):
     backend.teardown_board(board)
 
 
-def test_constructor_board(board):
+def test_constructor_board(board, df_csv, tmp_cache):
     prot = board.fs.protocol
 
     fs_name = prot if isinstance(prot, str) else prot[0]
@@ -58,9 +110,20 @@ def test_constructor_board(board):
         con_name = fs_name
 
     board = getattr(c, f"board_{con_name}")(board.board)
+    df = board.pin_read("df_csv")
 
-    # check cache
     # check data
+    assert_frame_equal(df, df_csv)
+
+    # check cache
+    options = list(tmp_cache.glob("s3_*"))
+    assert len(options) == 1
+
+    cache_dir = options[0]
+    res = list(cache_dir.rglob("**/*.csv"))
+    assert len(res) == 1
+
+    check_cache_file_path(res[0], cache_dir)
 
 
 # Board particulars ===========================================================