Skip to content

Commit

Permalink
Fix fsspec tests in ci (#1635)
Browse files Browse the repository at this point in the history
* fix warnings + typoes

* Raise on NotImplementedError in fsspec

* unskip test
  • Loading branch information
Wauplin authored Sep 4, 2023
1 parent 0ac546b commit 882be57
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 11 deletions.
22 changes: 20 additions & 2 deletions src/huggingface_hub/hf_file_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,24 @@ def _repo_and_revision_exist(
self._repo_and_revision_exists_cache[(repo_type, repo_id, None)] = True, None
return self._repo_and_revision_exists_cache[(repo_type, repo_id, revision)]

def exists(self, path, **kwargs):
"""Is there a file at the given path
Exact same implementation as in fsspec except that instead of catching all exceptions, we only catch when it's
not a `NotImplementedError` (which we do want to raise). Catching a `NotImplementedError` can lead to undesired
behavior.
Adapted from https://github.com/fsspec/filesystem_spec/blob/f5d24b80a0768bf07a113647d7b4e74a3a2999e0/fsspec/spec.py#L649C1-L656C25
"""
try:
self.info(path, **kwargs)
return True
except Exception as e: # noqa: E722
if isinstance(e, NotImplementedError):
raise
# any exception allowed bar FileNotFoundError?
return False

def resolve_path(self, path: str, revision: Optional[str] = None) -> HfFileSystemResolvedPath:
def _align_revision_in_path_with_revision(
revision_in_path: Optional[str], revision: Optional[str]
Expand All @@ -130,7 +148,7 @@ def _align_revision_in_path_with_revision(
elif path.split("/")[0] + "/" in REPO_TYPES_URL_PREFIXES.values():
if "/" not in path:
# can't list repositories at the repository type level
raise NotImplementedError("Acces to repositories lists is not implemented.")
raise NotImplementedError("Access to repositories lists is not implemented.")
repo_type, path = path.split("/", 1)
repo_type = REPO_TYPES_MAPPING[repo_type]
else:
Expand Down Expand Up @@ -173,7 +191,7 @@ def _align_revision_in_path_with_revision(
revision = _align_revision_in_path_with_revision(revision_in_path, revision)
repo_and_revision_exist, _ = self._repo_and_revision_exist(repo_type, repo_id, revision)
if not repo_and_revision_exist:
raise NotImplementedError("Acces to repositories lists is not implemented.")
raise NotImplementedError("Access to repositories lists is not implemented.")

revision = revision if revision is not None else DEFAULT_REVISION
return HfFileSystemResolvedPath(repo_type, repo_id, revision, path_in_repo)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_file_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ def test_with_local_dir_and_no_symlinks_and_overwrite(self) -> None:
local_dir_use_symlinks=False,
)
self.assertFalse(config_path.is_symlink())
self.assertNotEquals(config_path.read_text(), "this will be overwritten")
self.assertNotEqual(config_path.read_text(), "this will be overwritten")


@pytest.mark.usefixtures("fx_cache_dir")
Expand Down
10 changes: 5 additions & 5 deletions tests/test_hf_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2474,7 +2474,7 @@ def test_manage_variables(self) -> None:

# Returning all variables created
variables = self.api.get_space_variables(self.repo_id)
self.assertEquals(len(variables), 3)
self.assertEqual(len(variables), 3)

def test_space_runtime(self) -> None:
runtime = self.api.get_space_runtime(self.repo_id)
Expand Down Expand Up @@ -2940,14 +2940,14 @@ def test_list_commits_on_main(self) -> None:
commits = self.api.list_repo_commits(self.repo_id)

# "on_pr" commit not returned
self.assertEquals(len(commits), 3)
self.assertEqual(len(commits), 3)
self.assertTrue(all("on_pr" not in commit.title for commit in commits))

# USER is always the author
self.assertTrue(all(commit.authors == [USER] for commit in commits))

# latest commit first
self.assertEquals(commits[0].title, "Upload on_main.txt with huggingface_hub")
self.assertEqual(commits[0].title, "Upload on_main.txt with huggingface_hub")

# Formatted field not returned by default
for commit in commits:
Expand All @@ -2958,9 +2958,9 @@ def test_list_commits_on_pr(self) -> None:
commits = self.api.list_repo_commits(self.repo_id, revision="refs/pr/1")

# "on_pr" commit returned but not the "on_main" one
self.assertEquals(len(commits), 3)
self.assertEqual(len(commits), 3)
self.assertTrue(all("on_main" not in commit.title for commit in commits))
self.assertEquals(commits[0].title, "Upload on_pr.txt with huggingface_hub")
self.assertEqual(commits[0].title, "Upload on_pr.txt with huggingface_hub")

def test_list_commits_include_formatted(self) -> None:
for commit in self.api.list_repo_commits(self.repo_id, formatted=True):
Expand Down
7 changes: 4 additions & 3 deletions tests/test_hf_file_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,11 @@ def test_resolve_path_with_non_matching_revisions():
@pytest.mark.parametrize("not_supported_path", ["", "foo", "datasets", "datasets/foo"])
def test_access_repositories_lists(not_supported_path):
fs = HfFileSystem()
with pytest.raises(NotImplementedError):
fs.info(not_supported_path)
with pytest.raises(NotImplementedError):
fs.ls(not_supported_path)
# Skip in test for now (see https://github.com/huggingface/huggingface_hub/pull/1635)
# with pytest.raises(NotImplementedError):
# fs.glob(not_supported_path + "/")
with pytest.raises(NotImplementedError):
fs.glob(not_supported_path + "/")
with pytest.raises(NotImplementedError):
fs.open(not_supported_path)

0 comments on commit 882be57

Please sign in to comment.