Use the same cache directory for remote and local files (#197)

Author: ariostas

src/skhep_testdata/data/__init__.py

@@ -0,0 +1,12 @@
+from __future__ import annotations
+
+from pathlib import Path
+
+
+def cache_path(cache_dir: Path | str | None = None) -> Path:
+    if cache_dir is None:
+        skhepdir = Path.home() / ".local" / "skhepdata"
+        skhepdir.mkdir(exist_ok=True, parents=True)
+        return skhepdir
+
+    return Path(cache_dir)

src/skhep_testdata/local_files.py

@@ -22,28 +22,21 @@
     known_files = {n.strip() for n in f if n.strip()}
 
 
-def _cache_path(cache_dir: str | None = None) -> Path:
-    if cache_dir is None:
-        skhepdir = Path.home() / ".local" / "skhepdata"
-        skhepdir.mkdir(exist_ok=True, parents=True)
-        return skhepdir
-
-    return Path(cache_dir)
-
-
 def data_path(
     filename: str, raise_missing: bool = True, cache_dir: str | None = None
 ) -> str:
     if remote_files.is_known_remote(filename):
-        return remote_files.remote_file(filename, raise_missing=raise_missing)
+        return remote_files.remote_file(
+            filename, cache_dir=cache_dir, raise_missing=raise_missing
+        )
 
     if filename not in known_files and raise_missing:
         raise FileNotFoundError(filename)
 
     filepath = DIR / "data" / filename
 
     if not filepath.exists() and filename in known_files:
-        cached_path = _cache_path(cache_dir) / filename
+        cached_path = data.cache_path(cache_dir) / filename
 
         if not cached_path.exists():
             # Currently get from main branch
@@ -60,7 +53,7 @@ def data_path(
 
 
 def download_all(cache_dir: str | None = None) -> None:
-    local_dir = _cache_path(cache_dir)
+    local_dir = data.cache_path(cache_dir)
 
     with tempfile.TemporaryFile() as f:
         with requests.get(zipurl, stream=True) as r:

src/skhep_testdata/remote_files.py

@@ -9,7 +9,7 @@
 
 import yaml
 
-_default_data_dir = Path(__file__).resolve().parent
+from . import data
 
 
 class RemoteDatasetList:
@@ -60,9 +60,9 @@ def make_all_dirs(path: str) -> None:
 
 
 def fetch_remote_dataset(
-    dataset_name: str, files: dict[str, str], url: str, data_dir: str
+    dataset_name: str, files: dict[str, str], url: str, cache_dir: str
 ) -> None:
-    dataset_dir = Path(data_dir) / dataset_name
+    dataset_dir = Path(cache_dir) / dataset_name
 
     writefile = dataset_dir / Path(url).name
     if writefile.exists():
@@ -72,9 +72,9 @@ def fetch_remote_dataset(
     logging.warning("Downloading %s", url)
     urlretrieve(url, str(writefile))
 
-    if tarfile.is_tarfile(writefile):
+    if tarfile.is_tarfile(str(writefile)):
         logging.warning("Extracting %s", writefile)
-        with tarfile.open(writefile) as tar:
+        with tarfile.open(str(writefile)) as tar:
             members = [tar.getmember(f) for f in files.values()]
             tar.extractall(str(dataset_dir), members)
 
@@ -93,16 +93,17 @@ def is_known_remote(filename: str) -> bool:
 
 
 def remote_file(
-    filename: str, data_dir: str | Path = _default_data_dir, raise_missing: bool = False
+    filename: str, cache_dir: str | Path | None = None, raise_missing: bool = False
 ) -> str:
+    cache_dir = data.cache_path(cache_dir)
     config = RemoteDatasetList.get_config_for_file(filename)
     if not config and raise_missing:
         msg = f"Unknown {filename} cannot be found"
         raise RuntimeError(msg)
 
-    path = Path(data_dir) / filename
+    path = Path(cache_dir) / filename
     if not path.is_file():
-        config["data_dir"] = str(data_dir)
+        config["cache_dir"] = str(cache_dir)
         fetch_remote_dataset(**config)  # type: ignore[arg-type]
 
     if not path.is_file() and raise_missing:

tests/test_local_files.py

@@ -32,10 +32,10 @@ def test_data_path_cached():
 
 
 def test_delegate_to_remote(monkeypatch, tmpdir):
-    def dummy_remote_file(filename, data_dir=None, raise_missing=False):
-        if not data_dir:
-            data_dir = str(tmpdir)
-        return str(Path(data_dir) / filename)
+    def dummy_remote_file(filename, cache_dir=None, raise_missing=False):
+        if not cache_dir:
+            cache_dir = str(tmpdir)
+        return str(Path(cache_dir) / filename)
 
     monkeypatch.setattr(skhtd.remote_files, "remote_file", dummy_remote_file)
     monkeypatch.setattr(skhtd.remote_files, "is_known_remote", lambda _: True)

tests/test_remote_files.py

@@ -37,5 +37,5 @@ def fake_urlretrieve(url, writefile):
 
     monkeypatch.setattr(skhtd.remote_files, "urlretrieve", fake_urlretrieve)
 
-    path = skhtd.remote_files.remote_file(good_file_1, data_dir=str(tmpdir))
+    path = skhtd.remote_files.remote_file(good_file_1, cache_dir=str(tmpdir))
     assert path == str(tmpdir / "dataset_1" / "file_1.root")