From 64be6e62b3917cfb4bf40f51b996103704f95d33 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 13:22:30 -0800 Subject: [PATCH 01/16] Allow relative paths in FCPath --- filecache/file_cache_path.py | 165 +++++++++++++++++----------------- tests/test_file_cache.py | 6 -- tests/test_file_cache_path.py | 32 ------- 3 files changed, 81 insertions(+), 122 deletions(-) diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index f0c7984..a174892 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -561,6 +561,13 @@ def is_absolute(self) -> bool: return FCPath._is_absolute(self._path) + def as_absolute(self) -> FCPath: + """Return the absolute version of this possibly-relative path.""" + + if FCPath._is_absolute(self._path): + return self + return FCPath(self.as_pathlib().expanduser().resolve(), copy_from=self) + def match(self, path_pattern: str | Path | FCPath) -> bool: """Return True if this path matches the given pattern. @@ -614,6 +621,22 @@ def _filecache_to_use(self) -> "FileCache": return _DEFAULT_FILECACHE return self._filecache + def _make_paths_absolute(self, + sub_path: Optional[StrOrPathOrSeqType]) -> str | list[str]: + if isinstance(sub_path, (list, tuple)): + new_sub_paths: list[str] = [] + for p in sub_path: + new_sub_path = FCPath._join(self._path, p) + if not FCPath._is_absolute(new_sub_path): + new_sub_path = (FCPath(Path(new_sub_path).expanduser().resolve()) + .as_posix()) + new_sub_paths.append(new_sub_path) + return new_sub_paths + new_sub_path = FCPath._join(self._path, sub_path) + if not FCPath._is_absolute(new_sub_path): + return FCPath(Path(new_sub_path).expanduser().resolve()).as_posix() + return new_sub_path + def get_local_path(self, sub_path: Optional[StrOrPathOrSeqType] = None, *, @@ -625,6 +648,9 @@ def get_local_path(self, Parameters: sub_path: The path of the file relative to this path. If not specified, this path is used. If `sub_path` is a list or tuple, all paths are processed. + If the resulting derived path is not absolute, it is assumed to be a + relative local path and is converted to an absolute path by expanding + usernames and resolving links. create_parents: If True, create all parent directories. This is useful when getting the local path of a file that will be uploaded. url_to_path: The function (or list of functions) that is used to translate @@ -658,26 +684,17 @@ def get_local_path(self, because a Path could be used for writing a file to upload. To facilitate this, a side effect of this call (if `create_parents` is True) is that the complete parent directory structure will be created for each returned Path. - - Raises: - ValueError: If the derived path is not absolute. """ - if isinstance(sub_path, (list, tuple)): - new_sub_paths = [FCPath._join(self._path, p) for p in sub_path] - if not all([FCPath._is_absolute(x) for x in new_sub_paths]): - raise ValueError( - f'Derived paths must be absolute, got {new_sub_paths}') + new_sub_path = self._make_paths_absolute(sub_path) + + if isinstance(new_sub_path, (list, tuple)): return self._filecache_to_use.get_local_path(cast(StrOrPathOrSeqType, - new_sub_paths), + new_sub_path), anonymous=self._anonymous, create_parents=create_parents, url_to_path=url_to_path) - new_sub_path = FCPath._join(self._path, sub_path) - if not FCPath._is_absolute(str(new_sub_path)): - raise ValueError( - f'Derived path must be absolute, got {new_sub_path}') return self._filecache_to_use.get_local_path(cast(StrOrPathOrSeqType, new_sub_path), anonymous=self._anonymous, @@ -695,7 +712,9 @@ def exists(self, Parameters: sub_path: The path of the file relative to this path. If not specified, this - path is used. + path is used. If the resulting derived path is not absolute, it is assumed + to be a relative local path and is converted to an absolute path by + expanding usernames and resolving links. bypass_cache: If False, check for the file first in the local cache, and if not found there then on the remote server. If True, only check on the remote server. @@ -732,29 +751,20 @@ def exists(self, still not be downloadable due to permissions. False if the file does not exist. This includes bad bucket or webserver names, lack of permission to examine a bucket's contents, etc. - - Raises: - ValueError: If the derived path is not absolute. """ nthreads = self._validate_nthreads(nthreads) - if isinstance(sub_path, (list, tuple)): - new_sub_paths = [FCPath._join(self._path, p) for p in sub_path] - if not all([FCPath._is_absolute(x) for x in new_sub_paths]): - raise ValueError( - f'Derived paths must be absolute, got {new_sub_paths}') + new_sub_path = self._make_paths_absolute(sub_path) + + if isinstance(new_sub_path, (list, tuple)): return self._filecache_to_use.exists(cast(StrOrPathOrSeqType, - new_sub_paths), + new_sub_path), bypass_cache=bypass_cache, nthreads=nthreads, anonymous=self._anonymous, url_to_path=url_to_path) - new_sub_path = FCPath._join(self._path, sub_path) - if not FCPath._is_absolute(str(new_sub_path)): - raise ValueError( - f'Derived path must be absolute, got {new_sub_path}') return self._filecache_to_use.exists(cast(StrOrPathOrSeqType, new_sub_path), bypass_cache=bypass_cache, @@ -762,7 +772,7 @@ def exists(self, url_to_path=url_to_path) def retrieve(self, - sub_path: Optional[StrOrSeqType] = None, + sub_path: Optional[StrOrPathOrSeqType] = None, *, lock_timeout: Optional[int] = None, nthreads: Optional[int] = None, @@ -775,7 +785,10 @@ def retrieve(self, sub_path: The path of the file relative to this path. If not specified, this path is used. If `sub_path` is a list or tuple, the complete list of files is retrieved. Depending on the storage location, this may be more - efficient because files can be downloaded in parallel. + efficient because files can be downloaded in parallel. If the resulting + derived path is not absolute, it is assumed to be a relative local path + and is converted to an absolute path by expanding usernames and resolving + links. nthreads: The maximum number of threads to use when doing multiple-file retrieval or upload. If None, use the default value given when this :class:`FCPath` was created. @@ -829,7 +842,6 @@ def retrieve(self, within the given timeout or, for a multi-file download, if we timed out waiting for other processes to download locked files, and exception_on_fail is True. - ValueError: If the derived path is not absolute. Notes: File download is normally an atomic operation; a program will never see a @@ -845,26 +857,20 @@ def retrieve(self, if lock_timeout is None: lock_timeout = self._lock_timeout + new_sub_path = self._make_paths_absolute(sub_path) + try: - if isinstance(sub_path, (list, tuple)): - new_sub_paths = [FCPath._join(self._path, p) for p in sub_path] - if not all([FCPath._is_absolute(x) for x in new_sub_paths]): - raise ValueError( - f'Derived paths must be absolute, got {new_sub_paths}') + if isinstance(new_sub_path, (list, tuple)): ret = self._filecache_to_use.retrieve(cast(StrOrPathOrSeqType, - new_sub_paths), + new_sub_path), anonymous=self._anonymous, lock_timeout=lock_timeout, nthreads=nthreads, exception_on_fail=exception_on_fail, url_to_path=url_to_path) else: - new_sub_path2 = FCPath._join(self._path, sub_path) - if not FCPath._is_absolute(str(new_sub_path2)): - raise ValueError( - f'Derived path must be absolute, got {new_sub_path2}') ret = self._filecache_to_use.retrieve(cast(StrOrPathOrSeqType, - new_sub_path2), + new_sub_path), anonymous=self._anonymous, lock_timeout=lock_timeout, exception_on_fail=exception_on_fail, @@ -876,7 +882,7 @@ def retrieve(self, return ret def upload(self, - sub_path: Optional[StrOrSeqType] = None, + sub_path: Optional[StrOrPathOrSeqType] = None, *, nthreads: Optional[int] = None, exception_on_fail: bool = True, @@ -888,7 +894,9 @@ def upload(self, sub_path: The path of the file relative to this path. If not specified, this path is used. If `sub_path` is a list or tuple, the complete list of files is uploaded. This may be more efficient because files can be uploaded in - parallel. + parallel. If the resulting derived path is not absolute, it is assumed to + be a relative local path and is converted to an absolute path by expanding + usernames and resolving links. nthreads: The maximum number of threads to use when doing multiple-file retrieval or upload. If None, use the default value given when this :class:`FileCache` was created. @@ -931,30 +939,23 @@ def upload(self, Raises: FileNotFoundError: If a file to upload does not exist or the upload failed, and exception_on_fail is True. - ValueError: If the derived path is not absolute. """ old_upload_counter = self._filecache_to_use.upload_counter nthreads = self._validate_nthreads(nthreads) + new_sub_path = self._make_paths_absolute(sub_path) + try: if isinstance(sub_path, (list, tuple)): - new_sub_paths = [FCPath._join(self._path, p) for p in sub_path] - if not all([FCPath._is_absolute(x) for x in new_sub_paths]): - raise ValueError( - f'Derived paths must be absolute, got {new_sub_paths}') ret = self._filecache_to_use.upload(cast(StrOrPathOrSeqType, - new_sub_paths), + new_sub_path), anonymous=self._anonymous, nthreads=nthreads, exception_on_fail=exception_on_fail, url_to_path=url_to_path) else: - new_sub_path = FCPath._join(self._path, sub_path) - if not FCPath._is_absolute(str(new_sub_path)): - raise ValueError( - f'Derived path must be absolute, got {new_sub_path}') ret = self._filecache_to_use.upload(cast(StrOrPathOrSeqType, new_sub_path), anonymous=self._anonymous, @@ -1043,7 +1044,10 @@ def unlink(self, sub_path: The path of the file relative to this path. If not specified, this path is used. If `sub_path` is a list or tuple, the complete list of files is retrieved. Depending on the storage location, this may be more - efficient because files can be downloaded in parallel. + efficient because files can be downloaded in parallel. If the resulting + derived path is not absolute, it is assumed to be a relative local path + and is converted to an absolute path by expanding usernames and resolving + links. missing_ok: True to ignore attempting to unlink a file that doesn't exist. nthreads: The maximum number of threads to use when doing multiple-file retrieval or upload. If None, use the default value given when this @@ -1081,31 +1085,25 @@ def unlink(self, See `pathlib.Path.unlink` for full documentation. """ - if isinstance(sub_path, (list, tuple)): - new_sub_paths = [FCPath._join(self._path, p) for p in sub_path] - if not all([FCPath._is_absolute(x) for x in new_sub_paths]): - raise ValueError( - f'Derived paths must be absolute, got {new_sub_paths}') - return self._filecache_to_use.unlink(cast(StrOrPathOrSeqType, - new_sub_paths), - missing_ok=missing_ok, - anonymous=self._anonymous, - nthreads=nthreads, - exception_on_fail=exception_on_fail, - url_to_path=url_to_path) - else: - new_sub_path2 = FCPath._join(self._path, sub_path) - if not FCPath._is_absolute(str(new_sub_path2)): - raise ValueError( - f'Derived path must be absolute, got {new_sub_path2}') + new_sub_path = self._make_paths_absolute(sub_path) + + if isinstance(new_sub_path, (list, tuple)): return self._filecache_to_use.unlink(cast(StrOrPathOrSeqType, - new_sub_path2), + new_sub_path), missing_ok=missing_ok, anonymous=self._anonymous, nthreads=nthreads, exception_on_fail=exception_on_fail, url_to_path=url_to_path) + return self._filecache_to_use.unlink(cast(StrOrPathOrSeqType, + new_sub_path), + missing_ok=missing_ok, + anonymous=self._anonymous, + nthreads=nthreads, + exception_on_fail=exception_on_fail, + url_to_path=url_to_path) + @property def download_counter(self) -> int: """The number of actual file downloads that have taken place.""" @@ -1290,32 +1288,31 @@ def rename(self, if not isinstance(target, FCPath): target = FCPath(target) + target = target.as_absolute() - if not self.is_absolute() or not target.is_absolute(): - raise ValueError('Source and target files must be absolute ' - f'{self.path!r} and f{target.path!r}') + src = self.as_absolute() - if self.is_local() != target.is_local(): + if src.is_local() != target.is_local(): raise ValueError('Unable to rename files between local and remote locations: ' - f'{self.path!r} and f{target.path!r}') + f'{src.path!r} and f{target.path!r}') - drive1, root1, subpath1 = FCPath._split_parts(self.path) + drive1, root1, subpath1 = FCPath._split_parts(src.path) drive2, root2, subpath2 = FCPath._split_parts(target.path) if drive1 != drive2 or root1 != root2: - raise ValueError('Unable to rename files across location: ' - f'{self.path!r} and f{target.path!r}') + raise ValueError('Unable to rename files across locations: ' + f'{src.path!r} and f{target.path!r}') - if self.is_local(): + if src.is_local(): # Local to local - just do an OS rename and be done with it target.parent.mkdir(parents=True, exist_ok=True) - self.as_pathlib().rename(target.as_pathlib()) + src.as_pathlib().rename(target.as_pathlib()) return target # Since you generally can't rename on a remote cloud location, first # download the file, then rename it locally, then upload it to the new name, # then delete the old name - local_path = cast(Path, self.retrieve()) + local_path = cast(Path, src.retrieve()) target_local_path = cast(Path, target.get_local_path()) local_path.rename(target_local_path) # Rename in the cache try: @@ -1323,7 +1320,7 @@ def rename(self, except Exception: target_local_path.unlink(missing_ok=True) raise - self.unlink(missing_ok=True) # Delete the old version + src.unlink(missing_ok=True) # Delete the old version return target diff --git a/tests/test_file_cache.py b/tests/test_file_cache.py index 8b745d7..5dac190 100644 --- a/tests/test_file_cache.py +++ b/tests/test_file_cache.py @@ -2365,12 +2365,6 @@ def test_rename_bad(): lpath1 = 'test_file1.txt' fpath1 = FCPath('/tmp/test_file1.txt') flpath1 = FCPath('test_file1.txt') - with pytest.raises(ValueError): - flpath1.rename(apath1) - with pytest.raises(ValueError): - fpath1.rename(flpath1) - with pytest.raises(ValueError): - fpath1.rename(lpath1) with pytest.raises(ValueError): gpath1.rename(spath1) with pytest.raises(ValueError): diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index 4a8c6ef..75f575b 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -676,38 +676,6 @@ def test_default_filecache(): assert p2.get_local_path() == p3.get_local_path() -def test_operations_relative_paths(): - p = FCPath('b/c') - with pytest.raises(ValueError): - p.get_local_path() - with pytest.raises(ValueError): - p.get_local_path('d') - with pytest.raises(ValueError): - p.get_local_path(['d', 'e']) - with pytest.raises(ValueError): - p.retrieve() - with pytest.raises(ValueError): - p.retrieve('d') - with pytest.raises(ValueError): - p.retrieve(['d', 'e']) - with pytest.raises(ValueError): - p.exists() - with pytest.raises(ValueError): - p.exists('d') - with pytest.raises(ValueError): - p.exists(['d', 'e']) - with pytest.raises(ValueError): - p.upload() - with pytest.raises(ValueError): - p.upload('d') - with pytest.raises(ValueError): - p.upload(['d', 'e']) - with pytest.raises(ValueError): - p.unlink('d') - with pytest.raises(ValueError): - p.unlink(['d', 'e']) - - def test_relative(): assert (FCPath(f'{GS_TEST_BUCKET_ROOT}/a/b/c.txt') .relative_to(f'{GS_TEST_BUCKET_ROOT}/a/b')) == FCPath('c.txt') From e2e12ff5bc41624402bba7771cc408223e86d454 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 13:25:36 -0800 Subject: [PATCH 02/16] Fix flake8 --- tests/test_file_cache.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_file_cache.py b/tests/test_file_cache.py index 5dac190..d5b480b 100644 --- a/tests/test_file_cache.py +++ b/tests/test_file_cache.py @@ -2361,10 +2361,7 @@ def test_rename_bad(): spath = FCPath(f'{S3_TEST_BUCKET_ROOT}/{uuid.uuid4()}') gpath1 = gpath / 'test_file1.txt' spath1 = spath / 'test_file1.txt' - apath1 = '/tmp/test_file1.txt' - lpath1 = 'test_file1.txt' fpath1 = FCPath('/tmp/test_file1.txt') - flpath1 = FCPath('test_file1.txt') with pytest.raises(ValueError): gpath1.rename(spath1) with pytest.raises(ValueError): From 7324d7c085d1215d0653a3aa4d3f3a25eea4f66f Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 13:42:14 -0800 Subject: [PATCH 03/16] Simplify --- filecache/file_cache_path.py | 70 +++++++++--------------------------- 1 file changed, 17 insertions(+), 53 deletions(-) diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index a174892..26568de 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -688,13 +688,6 @@ def get_local_path(self, new_sub_path = self._make_paths_absolute(sub_path) - if isinstance(new_sub_path, (list, tuple)): - return self._filecache_to_use.get_local_path(cast(StrOrPathOrSeqType, - new_sub_path), - anonymous=self._anonymous, - create_parents=create_parents, - url_to_path=url_to_path) - return self._filecache_to_use.get_local_path(cast(StrOrPathOrSeqType, new_sub_path), anonymous=self._anonymous, @@ -757,17 +750,10 @@ def exists(self, new_sub_path = self._make_paths_absolute(sub_path) - if isinstance(new_sub_path, (list, tuple)): - return self._filecache_to_use.exists(cast(StrOrPathOrSeqType, - new_sub_path), - bypass_cache=bypass_cache, - nthreads=nthreads, - anonymous=self._anonymous, - url_to_path=url_to_path) - return self._filecache_to_use.exists(cast(StrOrPathOrSeqType, new_sub_path), bypass_cache=bypass_cache, + nthreads=nthreads, anonymous=self._anonymous, url_to_path=url_to_path) @@ -860,21 +846,13 @@ def retrieve(self, new_sub_path = self._make_paths_absolute(sub_path) try: - if isinstance(new_sub_path, (list, tuple)): - ret = self._filecache_to_use.retrieve(cast(StrOrPathOrSeqType, - new_sub_path), - anonymous=self._anonymous, - lock_timeout=lock_timeout, - nthreads=nthreads, - exception_on_fail=exception_on_fail, - url_to_path=url_to_path) - else: - ret = self._filecache_to_use.retrieve(cast(StrOrPathOrSeqType, - new_sub_path), - anonymous=self._anonymous, - lock_timeout=lock_timeout, - exception_on_fail=exception_on_fail, - url_to_path=url_to_path) + ret = self._filecache_to_use.retrieve(cast(StrOrPathOrSeqType, + new_sub_path), + anonymous=self._anonymous, + lock_timeout=lock_timeout, + nthreads=nthreads, + exception_on_fail=exception_on_fail, + url_to_path=url_to_path) finally: self._download_counter += (self._filecache_to_use.download_counter - old_download_counter) @@ -948,19 +926,12 @@ def upload(self, new_sub_path = self._make_paths_absolute(sub_path) try: - if isinstance(sub_path, (list, tuple)): - ret = self._filecache_to_use.upload(cast(StrOrPathOrSeqType, - new_sub_path), - anonymous=self._anonymous, - nthreads=nthreads, - exception_on_fail=exception_on_fail, - url_to_path=url_to_path) - else: - ret = self._filecache_to_use.upload(cast(StrOrPathOrSeqType, - new_sub_path), - anonymous=self._anonymous, - exception_on_fail=exception_on_fail, - url_to_path=url_to_path) + ret = self._filecache_to_use.upload(cast(StrOrPathOrSeqType, + new_sub_path), + anonymous=self._anonymous, + nthreads=nthreads, + exception_on_fail=exception_on_fail, + url_to_path=url_to_path) finally: self._upload_counter += (self._filecache_to_use.upload_counter - old_upload_counter) @@ -1031,7 +1002,7 @@ def open(self, self.upload(sub_path, url_to_path=url_to_path) def unlink(self, - sub_path: Optional[StrOrSeqType] = None, + sub_path: Optional[StrOrPathOrSeqType] = None, *, missing_ok: bool = False, nthreads: Optional[int] = None, @@ -1085,16 +1056,9 @@ def unlink(self, See `pathlib.Path.unlink` for full documentation. """ - new_sub_path = self._make_paths_absolute(sub_path) + nthreads = self._validate_nthreads(nthreads) - if isinstance(new_sub_path, (list, tuple)): - return self._filecache_to_use.unlink(cast(StrOrPathOrSeqType, - new_sub_path), - missing_ok=missing_ok, - anonymous=self._anonymous, - nthreads=nthreads, - exception_on_fail=exception_on_fail, - url_to_path=url_to_path) + new_sub_path = self._make_paths_absolute(sub_path) return self._filecache_to_use.unlink(cast(StrOrPathOrSeqType, new_sub_path), From b81dc745e6db0b388b326ebf6b65cfe5872e88ee Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 13:52:23 -0800 Subject: [PATCH 04/16] Flake8 --- filecache/file_cache_path.py | 1 - 1 file changed, 1 deletion(-) diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index 26568de..0b8d9b2 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -23,7 +23,6 @@ from .file_cache import FileCache # Circular import from .file_cache_types import (StrOrPathOrSeqType, - StrOrSeqType, UrlToPathFuncOrSeqType) From 45919aa906d60c49dfe6f165cf63a5f92c3dc7b1 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 17:09:25 -0800 Subject: [PATCH 05/16] Add relative file tests --- tests/test_file_cache_path.py | 69 +++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index 75f575b..eba223e 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -6,6 +6,7 @@ from pathlib import Path, PurePath import platform import sys +import tempfile import uuid import pytest @@ -978,3 +979,71 @@ def test_bad_threads(): FCPath('http://bucket/a/b/c', nthreads='a') with pytest.raises(ValueError): FCPath('http://bucket/a/b/c', nthreads=-1) + + +def test_relative_paths(): + f_cur_dir = FCPath.cwd() + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = Path(temp_dir).expanduser().resolve() + os.chdir(str(temp_dir)) + f_temp_dir = FCPath(temp_dir) + with FileCache(cache_name=None) as fc: + rp1 = 'file1.txt' + rp2 = 'file2.txt' + ap1 = temp_dir / rp1 + ap2 = temp_dir / rp2 + frp1 = fc.new_path(rp1) + frp2 = fc.new_path(rp2) + fap1 = fc.new_path(ap1) + fap2 = fc.new_path(ap2) + assert fap1.get_local_path() == ap1 + assert fap2.get_local_path() == ap2 + assert frp1.get_local_path() == ap1 + assert frp2.get_local_path() == ap2 + assert FCPath('').get_local_path([rp1, rp2]) == [ap1, ap2] + + frp2.touch() + + assert not fap1.exists() + assert fap2.exists() + assert not frp1.exists() + assert frp2.exists() + assert FCPath('').exists([rp1, rp2]) == [False, True] + + with pytest.raises(FileNotFoundError): + frp1.retrieve() + assert frp2.retrieve() == ap2 + ret = FCPath('').retrieve([rp1, rp2], exception_on_fail=False) + assert isinstance(ret[0], FileNotFoundError) + assert ret[1] == ap2 + + with pytest.raises(FileNotFoundError): + frp1.upload() + assert frp2.upload() == ap2 + ret = FCPath('').upload([rp1, rp2], exception_on_fail=False) + assert isinstance(ret[0], FileNotFoundError) + assert ret[1] == ap2 + + assert ap2.exists() + frp2.unlink() + assert not ap2.exists() + + frp1.touch() + frp2.touch() + assert ap1.exists() + assert ap2.exists() + FCPath('').unlink([rp1, rp2]) + assert not ap1.exists() + assert not ap2.exists() + + frp1.touch() + frp1.rename(frp2) + assert not ap1.exists() + assert ap2.exists() + + frp1.touch() + frp1.rename(rp2) + assert not ap1.exists() + assert ap2.exists() + + os.chdir(f_cur_dir.as_posix()) From e3e5a742c0858523a9e2259cda97a90aa25a1f72 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 17:10:03 -0800 Subject: [PATCH 06/16] tests --- tests/test_file_cache_path.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index eba223e..64e88c7 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -986,7 +986,6 @@ def test_relative_paths(): with tempfile.TemporaryDirectory() as temp_dir: temp_dir = Path(temp_dir).expanduser().resolve() os.chdir(str(temp_dir)) - f_temp_dir = FCPath(temp_dir) with FileCache(cache_name=None) as fc: rp1 = 'file1.txt' rp2 = 'file2.txt' From 4832a1d84375e7804087a74de2f70ba20e323365 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 17:33:54 -0800 Subject: [PATCH 07/16] Tests --- tests/test_file_cache_path.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index 64e88c7..b7b87d2 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -983,18 +983,21 @@ def test_bad_threads(): def test_relative_paths(): f_cur_dir = FCPath.cwd() - with tempfile.TemporaryDirectory() as temp_dir: - temp_dir = Path(temp_dir).expanduser().resolve() - os.chdir(str(temp_dir)) - with FileCache(cache_name=None) as fc: - rp1 = 'file1.txt' - rp2 = 'file2.txt' - ap1 = temp_dir / rp1 - ap2 = temp_dir / rp2 - frp1 = fc.new_path(rp1) - frp2 = fc.new_path(rp2) - fap1 = fc.new_path(ap1) - fap2 = fc.new_path(ap2) + # We put the files in the actual repo because we always have write acccess to the + # repo, while the Windows GitHub runner doesn't seem to be able to rename files in + # a temporary directory. + temp_dir = Path(__file__).resolve().parent + os.chdir(str(temp_dir)) + with FileCache(cache_name=None) as fc: + rp1 = '_test_file1.txt' + rp2 = '_test_file2.txt' + ap1 = temp_dir / rp1 + ap2 = temp_dir / rp2 + frp1 = fc.new_path(rp1) + frp2 = fc.new_path(rp2) + fap1 = fc.new_path(ap1) + fap2 = fc.new_path(ap2) + try: assert fap1.get_local_path() == ap1 assert fap2.get_local_path() == ap2 assert frp1.get_local_path() == ap1 @@ -1045,4 +1048,8 @@ def test_relative_paths(): assert not ap1.exists() assert ap2.exists() + finally: + ap1.unlink(missing_ok=True) + ap2.unlink(missing_ok=True) + os.chdir(f_cur_dir.as_posix()) From ab2d49b79cd84cfcb414276e5610e62c08b33700 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 17:45:37 -0800 Subject: [PATCH 08/16] tests --- tests/test_file_cache_path.py | 47 +++++++++++++++-------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index b7b87d2..ca5032d 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -983,21 +983,18 @@ def test_bad_threads(): def test_relative_paths(): f_cur_dir = FCPath.cwd() - # We put the files in the actual repo because we always have write acccess to the - # repo, while the Windows GitHub runner doesn't seem to be able to rename files in - # a temporary directory. - temp_dir = Path(__file__).resolve().parent - os.chdir(str(temp_dir)) - with FileCache(cache_name=None) as fc: - rp1 = '_test_file1.txt' - rp2 = '_test_file2.txt' - ap1 = temp_dir / rp1 - ap2 = temp_dir / rp2 - frp1 = fc.new_path(rp1) - frp2 = fc.new_path(rp2) - fap1 = fc.new_path(ap1) - fap2 = fc.new_path(ap2) - try: + with tempfile.TemporaryDirectory() as temp_dir: + temp_dir = Path(temp_dir).expanduser().resolve() + os.chdir(str(temp_dir)) + with FileCache(cache_name=None) as fc: + rp1 = 'file1.txt' + rp2 = 'file2.txt' + ap1 = temp_dir / rp1 + ap2 = temp_dir / rp2 + frp1 = fc.new_path(rp1) + frp2 = fc.new_path(rp2) + fap1 = fc.new_path(ap1) + fap2 = fc.new_path(ap2) assert fap1.get_local_path() == ap1 assert fap2.get_local_path() == ap2 assert frp1.get_local_path() == ap1 @@ -1038,18 +1035,14 @@ def test_relative_paths(): assert not ap1.exists() assert not ap2.exists() - frp1.touch() - frp1.rename(frp2) - assert not ap1.exists() - assert ap2.exists() - - frp1.touch() - frp1.rename(rp2) - assert not ap1.exists() - assert ap2.exists() + # frp1.touch() + # frp1.rename(frp2) + # assert not ap1.exists() + # assert ap2.exists() - finally: - ap1.unlink(missing_ok=True) - ap2.unlink(missing_ok=True) + # frp1.touch() + # frp1.rename(rp2) + # assert not ap1.exists() + # assert ap2.exists() os.chdir(f_cur_dir.as_posix()) From 5f10a89ac8cde4bce1d008f1a4e9bb5593969162 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 17:51:04 -0800 Subject: [PATCH 09/16] tests --- tests/test_file_cache_path.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index ca5032d..a1668de 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -1045,4 +1045,4 @@ def test_relative_paths(): # assert not ap1.exists() # assert ap2.exists() - os.chdir(f_cur_dir.as_posix()) + os.chdir(f_cur_dir.as_posix()) From 80637e3da0b6f7e590390bb1b7389fedef09687c Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 17:55:04 -0800 Subject: [PATCH 10/16] tests --- tests/test_file_cache_path.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index a1668de..3eb78dd 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -1035,14 +1035,14 @@ def test_relative_paths(): assert not ap1.exists() assert not ap2.exists() - # frp1.touch() - # frp1.rename(frp2) - # assert not ap1.exists() - # assert ap2.exists() - - # frp1.touch() - # frp1.rename(rp2) - # assert not ap1.exists() - # assert ap2.exists() + frp1.touch() + frp1.rename(frp2) + assert not ap1.exists() + assert ap2.exists() + + frp1.touch() + frp1.rename(rp2) + assert not ap1.exists() + assert ap2.exists() os.chdir(f_cur_dir.as_posix()) From 961548da8811fcd5ee00e7a5b867e67ed36bf390 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 18:04:04 -0800 Subject: [PATCH 11/16] tests --- .github/workflows/run-tests.yml | 6 +- filecache/file_cache_path.py | 2 +- tests/test_file_cache_path.py | 124 ++++++++++++++++---------------- 3 files changed, 68 insertions(+), 64 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 36448f6..c8dbae8 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -42,8 +42,10 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest, macos-latest, windows-latest] - python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] + # os: [ubuntu-latest, macos-latest, windows-latest] + # python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] + os: [windows-latest] + python-version: ['3.9'] fail-fast: false permissions: contents: read diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index 0b8d9b2..8005876 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -686,7 +686,7 @@ def get_local_path(self, """ new_sub_path = self._make_paths_absolute(sub_path) - + print('XXXXX', new_sub_path) return self._filecache_to_use.get_local_path(cast(StrOrPathOrSeqType, new_sub_path), anonymous=self._anonymous, diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index 3eb78dd..77b5ed6 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -985,64 +985,66 @@ def test_relative_paths(): f_cur_dir = FCPath.cwd() with tempfile.TemporaryDirectory() as temp_dir: temp_dir = Path(temp_dir).expanduser().resolve() - os.chdir(str(temp_dir)) - with FileCache(cache_name=None) as fc: - rp1 = 'file1.txt' - rp2 = 'file2.txt' - ap1 = temp_dir / rp1 - ap2 = temp_dir / rp2 - frp1 = fc.new_path(rp1) - frp2 = fc.new_path(rp2) - fap1 = fc.new_path(ap1) - fap2 = fc.new_path(ap2) - assert fap1.get_local_path() == ap1 - assert fap2.get_local_path() == ap2 - assert frp1.get_local_path() == ap1 - assert frp2.get_local_path() == ap2 - assert FCPath('').get_local_path([rp1, rp2]) == [ap1, ap2] - - frp2.touch() - - assert not fap1.exists() - assert fap2.exists() - assert not frp1.exists() - assert frp2.exists() - assert FCPath('').exists([rp1, rp2]) == [False, True] - - with pytest.raises(FileNotFoundError): - frp1.retrieve() - assert frp2.retrieve() == ap2 - ret = FCPath('').retrieve([rp1, rp2], exception_on_fail=False) - assert isinstance(ret[0], FileNotFoundError) - assert ret[1] == ap2 - - with pytest.raises(FileNotFoundError): - frp1.upload() - assert frp2.upload() == ap2 - ret = FCPath('').upload([rp1, rp2], exception_on_fail=False) - assert isinstance(ret[0], FileNotFoundError) - assert ret[1] == ap2 - - assert ap2.exists() - frp2.unlink() - assert not ap2.exists() - - frp1.touch() - frp2.touch() - assert ap1.exists() - assert ap2.exists() - FCPath('').unlink([rp1, rp2]) - assert not ap1.exists() - assert not ap2.exists() - - frp1.touch() - frp1.rename(frp2) - assert not ap1.exists() - assert ap2.exists() - - frp1.touch() - frp1.rename(rp2) - assert not ap1.exists() - assert ap2.exists() - - os.chdir(f_cur_dir.as_posix()) + try: + os.chdir(str(temp_dir)) + with FileCache(cache_name=None) as fc: + rp1 = 'file1.txt' + rp2 = 'file2.txt' + ap1 = temp_dir / rp1 + ap2 = temp_dir / rp2 + frp1 = fc.new_path(rp1) + frp2 = fc.new_path(rp2) + fap1 = fc.new_path(ap1) + fap2 = fc.new_path(ap2) + assert fap1.get_local_path() == ap1 + assert fap2.get_local_path() == ap2 + assert frp1.get_local_path() == ap1 + assert frp2.get_local_path() == ap2 + assert FCPath('').get_local_path([rp1, rp2]) == [ap1, ap2] + + frp2.touch() + + assert not fap1.exists() + assert fap2.exists() + assert not frp1.exists() + assert frp2.exists() + assert FCPath('').exists([rp1, rp2]) == [False, True] + + with pytest.raises(FileNotFoundError): + frp1.retrieve() + assert frp2.retrieve() == ap2 + ret = FCPath('').retrieve([rp1, rp2], exception_on_fail=False) + assert isinstance(ret[0], FileNotFoundError) + assert ret[1] == ap2 + + with pytest.raises(FileNotFoundError): + frp1.upload() + assert frp2.upload() == ap2 + ret = FCPath('').upload([rp1, rp2], exception_on_fail=False) + assert isinstance(ret[0], FileNotFoundError) + assert ret[1] == ap2 + + assert ap2.exists() + frp2.unlink() + assert not ap2.exists() + + frp1.touch() + frp2.touch() + assert ap1.exists() + assert ap2.exists() + FCPath('').unlink([rp1, rp2]) + assert not ap1.exists() + assert not ap2.exists() + + frp1.touch() + frp1.rename(frp2) + assert not ap1.exists() + assert ap2.exists() + + frp1.touch() + frp1.rename(rp2) + assert not ap1.exists() + assert ap2.exists() + + finally: + os.chdir(f_cur_dir.as_posix()) From 6a5e6051de12f3183df56ee2fbbbd604f83b6362 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 18:09:18 -0800 Subject: [PATCH 12/16] tests --- filecache/file_cache_path.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index 8005876..334c677 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -223,7 +223,7 @@ def _split(path: str) -> tuple[str, str]: @staticmethod def _is_absolute(path: str) -> bool: """Check if a path string is an absolute path.""" - + print(path, FCPath._split_parts(path)) return FCPath._split_parts(path)[1] == '/' @staticmethod @@ -686,7 +686,7 @@ def get_local_path(self, """ new_sub_path = self._make_paths_absolute(sub_path) - print('XXXXX', new_sub_path) + return self._filecache_to_use.get_local_path(cast(StrOrPathOrSeqType, new_sub_path), anonymous=self._anonymous, From 71b98bf15f01c5c2e4fb629c2f276251f98d11ac Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 18:32:45 -0800 Subject: [PATCH 13/16] tests --- filecache/file_cache_path.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index 334c677..7d38325 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -223,7 +223,7 @@ def _split(path: str) -> tuple[str, str]: @staticmethod def _is_absolute(path: str) -> bool: """Check if a path string is an absolute path.""" - print(path, FCPath._split_parts(path)) + return FCPath._split_parts(path)[1] == '/' @staticmethod @@ -626,14 +626,20 @@ def _make_paths_absolute(self, new_sub_paths: list[str] = [] for p in sub_path: new_sub_path = FCPath._join(self._path, p) + print('A', new_sub_path) if not FCPath._is_absolute(new_sub_path): new_sub_path = (FCPath(Path(new_sub_path).expanduser().resolve()) .as_posix()) + print('B', new_sub_path) new_sub_paths.append(new_sub_path) + print('C', new_sub_paths) return new_sub_paths new_sub_path = FCPath._join(self._path, sub_path) + print('D', new_sub_path) if not FCPath._is_absolute(new_sub_path): + print('E', FCPath(Path(new_sub_path).expanduser().resolve()).as_posix()) return FCPath(Path(new_sub_path).expanduser().resolve()).as_posix() + print('F', new_sub_path) return new_sub_path def get_local_path(self, From 0492e8f3eeb5a715bcf8986a6311e7a4c67a5952 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 18:40:55 -0800 Subject: [PATCH 14/16] tests --- filecache/file_cache_path.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index 7d38325..a9fa603 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -628,7 +628,8 @@ def _make_paths_absolute(self, new_sub_path = FCPath._join(self._path, p) print('A', new_sub_path) if not FCPath._is_absolute(new_sub_path): - new_sub_path = (FCPath(Path(new_sub_path).expanduser().resolve()) + new_sub_path = (FCPath(Path(new_sub_path) + .expanduser().absolute().resolve()) .as_posix()) print('B', new_sub_path) new_sub_paths.append(new_sub_path) @@ -637,8 +638,9 @@ def _make_paths_absolute(self, new_sub_path = FCPath._join(self._path, sub_path) print('D', new_sub_path) if not FCPath._is_absolute(new_sub_path): - print('E', FCPath(Path(new_sub_path).expanduser().resolve()).as_posix()) - return FCPath(Path(new_sub_path).expanduser().resolve()).as_posix() + print('E', FCPath(Path(new_sub_path).expanduser().absolute().resolve()).as_posix()) + return (FCPath(Path(new_sub_path).expanduser().absolute().resolve()) + .as_posix()) print('F', new_sub_path) return new_sub_path From 2b5594783e981fcd712c5ab7a7755f52a05c6e8e Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 18:45:48 -0800 Subject: [PATCH 15/16] tests --- .github/workflows/run-tests.yml | 6 ++---- filecache/file_cache_path.py | 12 ++++-------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index c8dbae8..36448f6 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -42,10 +42,8 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - # os: [ubuntu-latest, macos-latest, windows-latest] - # python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] - os: [windows-latest] - python-version: ['3.9'] + os: [ubuntu-latest, macos-latest, windows-latest] + python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] fail-fast: false permissions: contents: read diff --git a/filecache/file_cache_path.py b/filecache/file_cache_path.py index a9fa603..01d9723 100644 --- a/filecache/file_cache_path.py +++ b/filecache/file_cache_path.py @@ -565,7 +565,8 @@ def as_absolute(self) -> FCPath: if FCPath._is_absolute(self._path): return self - return FCPath(self.as_pathlib().expanduser().resolve(), copy_from=self) + return FCPath(self.as_pathlib().expanduser().absolute().resolve(), + copy_from=self) def match(self, path_pattern: str | Path | FCPath) -> bool: @@ -626,22 +627,16 @@ def _make_paths_absolute(self, new_sub_paths: list[str] = [] for p in sub_path: new_sub_path = FCPath._join(self._path, p) - print('A', new_sub_path) if not FCPath._is_absolute(new_sub_path): new_sub_path = (FCPath(Path(new_sub_path) .expanduser().absolute().resolve()) .as_posix()) - print('B', new_sub_path) new_sub_paths.append(new_sub_path) - print('C', new_sub_paths) return new_sub_paths new_sub_path = FCPath._join(self._path, sub_path) - print('D', new_sub_path) if not FCPath._is_absolute(new_sub_path): - print('E', FCPath(Path(new_sub_path).expanduser().absolute().resolve()).as_posix()) return (FCPath(Path(new_sub_path).expanduser().absolute().resolve()) .as_posix()) - print('F', new_sub_path) return new_sub_path def get_local_path(self, @@ -1585,7 +1580,8 @@ def resolve(self, """ if self.is_local(): - return FCPath(self.as_pathlib().resolve(strict=strict), copy_from=self) + return FCPath(self.as_pathlib().absolute().resolve(strict=strict), + copy_from=self) return self From d72d8b6cf7e29ab6ba7c1b2864b21df37baa26c7 Mon Sep 17 00:00:00 2001 From: Robert French Date: Mon, 6 Jan 2025 18:51:41 -0800 Subject: [PATCH 16/16] tests --- tests/test_file_cache_path.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_file_cache_path.py b/tests/test_file_cache_path.py index 77b5ed6..a113efa 100644 --- a/tests/test_file_cache_path.py +++ b/tests/test_file_cache_path.py @@ -1040,11 +1040,13 @@ def test_relative_paths(): frp1.rename(frp2) assert not ap1.exists() assert ap2.exists() + ap2.unlink() frp1.touch() frp1.rename(rp2) assert not ap1.exists() assert ap2.exists() + ap2.unlink() finally: os.chdir(f_cur_dir.as_posix())