Repository: huggingface/huggingface_hub
Pull request #1163 (2022-11-07 16:17:50)

Delete folder with commit endpoint
Resolves https://github.com/huggingface/huggingface_hub/issues/1151 following server-side implementation (https://github.com/huggingface/moon-landing/pull/3826 (internal link)).

`CommitOperationDelete` has now a different behavior depending on if the path is a file or a folder. By default, it will try to guess it automatically by looking at a trainling "/" on the path. Otherwise, it is possible to explicitly set `is_folder=True`. In case of a "file not found error" (most likely due to a folder passed as a file), a custom message is added to the `EntryNotFoundError`.

Also implements a high-level `delete_folder`, same as existing `delete_file`.

Added tests and updated the documentation. 


HuggingFaceDocBuilderDev: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1163). All of your documentation changes will be reflected on that endpoint.
codecov[bot]: # [Codecov](https://codecov.io/gh/huggingface/huggingface_hub/pull/1163?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) Report
Base: **84.57**% // Head: **84.62**% // Increases project coverage by **`+0.05%`** :tada:
> Coverage data is based on head [(`3f37b7a`)](https://codecov.io/gh/huggingface/huggingface_hub/pull/1163?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) compared to base [(`9ccdf02`)](https://codecov.io/gh/huggingface/huggingface_hub/commit/9ccdf02f50c7fcb9c0ad880dcd934fcf7bc4359c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1163      +/-   ##
==========================================
+ Coverage   84.57%   84.62%   +0.05%     
==========================================
  Files          42       42              
  Lines        4182     4196      +14     
==========================================
+ Hits         3537     3551      +14     
  Misses        645      645              
```


| [Impacted Files](https://codecov.io/gh/huggingface/huggingface_hub/pull/1163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface) | Coverage Δ | |
|---|---|---|
| [src/huggingface\_hub/\_commit\_api.py](https://codecov.io/gh/huggingface/huggingface_hub/pull/1163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface#diff-c3JjL2h1Z2dpbmdmYWNlX2h1Yi9fY29tbWl0X2FwaS5weQ==) | `93.18% <100.00%> (+0.32%)` | :arrow_up: |
| [src/huggingface\_hub/hf\_api.py](https://codecov.io/gh/huggingface/huggingface_hub/pull/1163/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface#diff-c3JjL2h1Z2dpbmdmYWNlX2h1Yi9oZl9hcGkucHk=) | `87.69% <100.00%> (+0.12%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/huggingface/huggingface_hub/pull/1163?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=huggingface).


Patch:
diff --git a/docs/source/how-to-discussions-and-pull-requests.mdx b/docs/source/how-to-discussions-and-pull-requests.mdx
index d6ecab93..353deeda 100644
--- a/docs/source/how-to-discussions-and-pull-requests.mdx
+++ b/docs/source/how-to-discussions-and-pull-requests.mdx
@@ -76,6 +76,7 @@ set the `create_pr` parameter to `True`. This parameter is also available on oth
     * [`upload_file`]
     * [`upload_folder`]
     * [`delete_file`]
+    * [`delete_folder`]
     * [`metadata_update`]
 
 ```python
diff --git a/docs/source/how-to-upstream.mdx b/docs/source/how-to-upstream.mdx
index b73614d4..061c79bf 100644
--- a/docs/source/how-to-upstream.mdx
+++ b/docs/source/how-to-upstream.mdx
@@ -78,16 +78,16 @@ If both `allow_patterns` and `ignore_patterns` are provided, both constraints ap
 
 If you want to work at a commit-level, use the [`create_commit`] function directly. There are two types of operations supported by [`create_commit`]:
 
-- `CommitOperationAdd` uploads a file to the Hub. If the file already exists, the file contents are overwritten. This operation accepts two arguments:
+- [`CommitOperationAdd`] uploads a file to the Hub. If the file already exists, the file contents are overwritten. This operation accepts two arguments:
 
   - `path_in_repo`: the repository path to upload a file to.
   - `path_or_fileobj`: either a path to a file on your filesystem or a file-like object. This is the content of the file to upload to the Hub.
 
-- `CommitOperationDelete` removes a file from a repository. This operation accepts `path_in_repo` as an argument.
+- [`CommitOperationDelete`] removes a file or a folder from a repository. This operation accepts `path_in_repo` as an argument.
 
 For example, if you want to upload two files and delete a file in a Hub repository:
 
-1. Use the appropriate `CommitOperation` to add and delete a file:
+1. Use the appropriate `CommitOperation` to add or delete a file and to delete a folder:
 
 ```py
 >>> from huggingface_hub import HfApi, CommitOperationAdd, CommitOperationDelete
@@ -96,6 +96,7 @@ For example, if you want to upload two files and delete a file in a Hub reposito
 ...     CommitOperationAdd(path_in_repo="LICENSE.md", path_or_fileobj="~/repo/LICENSE.md"),
 ...     CommitOperationAdd(path_in_repo="weights.h5", path_or_fileobj="~/repo/weights-final.h5"),
 ...     CommitOperationDelete(path_in_repo="old-weights.h5"),
+...     CommitOperationDelete(path_in_repo="logs/"),
 ... ]
 ```
 
@@ -112,6 +113,7 @@ For example, if you want to upload two files and delete a file in a Hub reposito
 In addition to [`upload_file`] and [`upload_folder`], the following functions also use [`create_commit`] under the hood:
 
 - [`delete_file`] deletes a single file from a repository on the Hub.
+- [`delete_folder`] deletes an entire folder from a repository on the Hub.
 - [`metadata_update`] updates a repository's metadata.
 
 For more detailed information, take a look at the [`HfApi`] reference.
diff --git a/setup.py b/setup.py
index 2d5fa4e2..4ccd9281 100644
--- a/setup.py
+++ b/setup.py
@@ -66,7 +66,7 @@ extras["quality"] = [
     "flake8>=3.8.3",
     "flake8-bugbear",
     "isort>=5.5.4",
-    "mypy",
+    "mypy==0.982",
 ]
 
 extras["all"] = extras["testing"] + extras["quality"] + extras["typing"]
diff --git a/src/huggingface_hub/README.md b/src/huggingface_hub/README.md
index 076667e5..df6e9aae 100644
--- a/src/huggingface_hub/README.md
+++ b/src/huggingface_hub/README.md
@@ -89,6 +89,7 @@ single file to a repo or listing models from the Hub, you'll find helpers in
 * `create_commit()`
 * `upload_file()`
 * `delete_file()`
+* `delete_folder()`
 
 Those API utilities are also exposed through the `huggingface-cli` CLI:
 
diff --git a/src/huggingface_hub/__init__.py b/src/huggingface_hub/__init__.py
index 4f89f6e9..08f1fc26 100644
--- a/src/huggingface_hub/__init__.py
+++ b/src/huggingface_hub/__init__.py
@@ -112,6 +112,7 @@ _SUBMOD_ATTRS = {
         "create_tag",
         "dataset_info",
         "delete_file",
+        "delete_folder",
         "delete_repo",
         "delete_tag",
         "edit_discussion_comment",
@@ -334,6 +335,7 @@ if TYPE_CHECKING:  # pragma: no cover
     from .hf_api import create_tag  # noqa: F401
     from .hf_api import dataset_info  # noqa: F401
     from .hf_api import delete_file  # noqa: F401
+    from .hf_api import delete_folder  # noqa: F401
     from .hf_api import delete_repo  # noqa: F401
     from .hf_api import delete_tag  # noqa: F401
     from .hf_api import edit_discussion_comment  # noqa: F401
diff --git a/src/huggingface_hub/_commit_api.py b/src/huggingface_hub/_commit_api.py
index d66d3e79..1b9a3c20 100644
--- a/src/huggingface_hub/_commit_api.py
+++ b/src/huggingface_hub/_commit_api.py
@@ -35,16 +35,31 @@ CommitOperationT = Union["CommitOperationAdd", "CommitOperationDelete"]
 @dataclass
 class CommitOperationDelete:
     """
-    Data structure holding necessary info to delete
-    a file from a repository on the Hub
+    Data structure holding necessary info to delete a file or a folder from a repository
+    on the Hub.
 
     Args:
         path_in_repo (`str`):
-            Relative filepath in the repo, for example:
-            `"checkpoints/1fec34a/weights.bin"`
+            Relative filepath in the repo, for example: `"checkpoints/1fec34a/weights.bin"`
+            for a file or `"checkpoints/1fec34a/"` for a folder.
+        is_folder (`bool` or `Literal["auto"]`, *optional*)
+            Whether the Delete Operation applies to a folder or not. If "auto", the path
+            type (file or folder) is guessed automatically by looking if path ends with
+            a "/" (folder) or not (file). To explicitly set the path type, you can set
+            `is_folder=True` or `is_folder=False`.
     """
 
     path_in_repo: str
+    is_folder: Union[bool, Literal["auto"]] = "auto"
+
+    def __post_init__(self):
+        if self.is_folder == "auto":
+            self.is_folder = self.path_in_repo.endswith("/")
+        if not isinstance(self.is_folder, bool):
+            raise ValueError(
+                "Wrong value for `is_folder`. Must be one of [`True`, `False`,"
+                f" `'auto'`]. Got '{self.is_folder}'."
+            )
 
 
 @dataclass
@@ -460,6 +475,9 @@ def prepare_commit_payload(
 
     # 4. Send deleted files, one per line
     yield from (
-        {"key": "deletedFile", "value": {"path": del_op.path_in_repo}}
+        {
+            "key": "deletedFolder" if del_op.is_folder else "deletedFile",
+            "value": {"path": del_op.path_in_repo},
+        }
         for del_op in deletions
     )
diff --git a/src/huggingface_hub/file_download.py b/src/huggingface_hub/file_download.py
index 92f041f4..c4334efb 100644
--- a/src/huggingface_hub/file_download.py
+++ b/src/huggingface_hub/file_download.py
@@ -234,7 +234,7 @@ def hf_hub_url(
     return HUGGINGFACE_CO_URL_TEMPLATE.format(
         repo_id=repo_id,
         revision=quote(revision, safe=""),
-        filename=filename,
+        filename=quote(filename),
     )
 
 
diff --git a/src/huggingface_hub/hf_api.py b/src/huggingface_hub/hf_api.py
index e6bf4a4a..a9198ce9 100644
--- a/src/huggingface_hub/hf_api.py
+++ b/src/huggingface_hub/hf_api.py
@@ -22,7 +22,7 @@ from typing import Any, BinaryIO, Dict, Iterable, Iterator, List, Optional, Tupl
 from urllib.parse import quote
 
 import requests
-from huggingface_hub.utils import RepositoryNotFoundError
+from huggingface_hub.utils import EntryNotFoundError, RepositoryNotFoundError
 from requests.exceptions import HTTPError
 
 from ._commit_api import (
@@ -1920,6 +1920,13 @@ class HfApi:
         except RepositoryNotFoundError as e:
             e.append_to_message(_CREATE_COMMIT_NO_REPO_ERROR_MESSAGE)
             raise
+        except EntryNotFoundError as e:
+            if len(deletions) > 0 and "A file with this name doesn't exist" in str(e):
+                e.append_to_message(
+                    "\nMake sure to differentiate file and folder paths in delete"
+                    " operations with a trailing '/' or using `is_folder=True/False`."
+                )
+            raise
 
         commit_data = commit_resp.json()
         return CommitInfo(
@@ -2327,6 +2334,73 @@ class HfApi:
             parent_commit=parent_commit,
         )
 
+    @validate_hf_hub_args
+    def delete_folder(
+        self,
+        path_in_repo: str,
+        repo_id: str,
+        *,
+        token: Optional[str] = None,
+        repo_type: Optional[str] = None,
+        revision: Optional[str] = None,
+        commit_message: Optional[str] = None,
+        commit_description: Optional[str] = None,
+        create_pr: Optional[bool] = None,
+        parent_commit: Optional[str] = None,
+    ) -> CommitInfo:
+        """
+        Deletes a folder in the given repo.
+
+        Simple wrapper around [`create_commit`] method.
+
+        Args:
+            path_in_repo (`str`):
+                Relative folder path in the repo, for example: `"checkpoints/1fec34a"`.
+            repo_id (`str`):
+                The repository from which the folder will be deleted, for example:
+                `"username/custom_transformers"`
+            token (`str`, *optional*):
+                Authentication token, obtained with `HfApi.login` method. Will default
+                to the stored token.
+            repo_type (`str`, *optional*):
+                Set to `"dataset"` or `"space"` if the folder is in a dataset or
+                space, `None` or `"model"` if in a model. Default is `None`.
+            revision (`str`, *optional*):
+                The git revision to commit from. Defaults to the head of the `"main"`
+                branch.
+            commit_message (`str`, *optional*):
+                The summary / title / first line of the generated commit. Defaults to
+                `f"Delete folder {path_in_repo} with huggingface_hub"`.
+            commit_description (`str` *optional*)
+                The description of the generated commit.
+            create_pr (`boolean`, *optional*):
+                Whether or not to create a Pull Request from `revision` with the changes.
+                Defaults to `False`.
+            parent_commit (`str`, *optional*):
+                The OID / SHA of the parent commit, as a hexadecimal string. Shorthands (7 first characters) are also supported.
+                If specified and `create_pr` is `False`, the commit will fail if `revision` does not point to `parent_commit`.
+                If specified and `create_pr` is `True`, the pull request will be created from `parent_commit`.
+                Specifying `parent_commit` ensures the repo has not changed before committing the changes, and can be
+                especially useful if the repo is updated / committed to concurrently.
+        """
+        return self.create_commit(
+            repo_id=repo_id,
+            repo_type=repo_type,
+            token=token,
+            operations=[
+                CommitOperationDelete(path_in_repo=path_in_repo, is_folder=True)
+            ],
+            revision=revision,
+            commit_message=(
+                commit_message
+                if commit_message is not None
+                else f"Delete folder {path_in_repo} with huggingface_hub"
+            ),
+            commit_description=commit_description,
+            create_pr=create_pr,
+            parent_commit=parent_commit,
+        )
+
     @validate_hf_hub_args
     def create_tag(
         self,
@@ -3312,6 +3386,7 @@ move_repo = api.move_repo
 upload_file = api.upload_file
 upload_folder = api.upload_folder
 delete_file = api.delete_file
+delete_folder = api.delete_folder
 create_tag = api.create_tag
 delete_tag = api.delete_tag
 get_full_repo_name = api.get_full_repo_name
diff --git a/src/huggingface_hub/templates/modelcard_template.md b/src/huggingface_hub/templates/modelcard_template.md
index e6f8ab80..d65cd201 100644
--- a/src/huggingface_hub/templates/modelcard_template.md
+++ b/src/huggingface_hub/templates/modelcard_template.md
@@ -73,7 +73,7 @@
 
 <!-- This section is meant to convey recommendations with respect to the bias, risk, and technical limitations. -->
 
-{{ bias_recommendations | default("Users (both direct and downstream) should be made aware of the risks, biases and limitations of the model. More information needed for further recomendations.", true)}}
+{{ bias_recommendations | default("Users (both direct and downstream) should be made aware of the risks, biases and limitations of the model. More information needed for further recommendations.", true)}}
 
 # Training Details
 


Test patch:
diff --git a/tests/test_commit_api.py b/tests/test_commit_api.py
new file mode 100644
index 00000000..1da4e2ca
--- /dev/null
+++ b/tests/test_commit_api.py
@@ -0,0 +1,48 @@
+import unittest
+
+from huggingface_hub._commit_api import CommitOperationDelete
+
+
+class TestCommitOperationDelete(unittest.TestCase):
+    def test_implicit_file(self):
+        self.assertFalse(CommitOperationDelete(path_in_repo="path/to/file").is_folder)
+        self.assertFalse(
+            CommitOperationDelete(path_in_repo="path/to/file.md").is_folder
+        )
+
+    def test_implicit_folder(self):
+        self.assertTrue(CommitOperationDelete(path_in_repo="path/to/folder/").is_folder)
+        self.assertTrue(
+            CommitOperationDelete(path_in_repo="path/to/folder.md/").is_folder
+        )
+
+    def test_explicit_file(self):
+        # Weird case: if user explicitly set as file (`is_folder`=False) but path has a
+        # trailing "/" => user input has priority
+        self.assertFalse(
+            CommitOperationDelete(
+                path_in_repo="path/to/folder/", is_folder=False
+            ).is_folder
+        )
+        self.assertFalse(
+            CommitOperationDelete(
+                path_in_repo="path/to/folder.md/", is_folder=False
+            ).is_folder
+        )
+
+    def test_explicit_folder(self):
+        # No need for the trailing "/" is `is_folder` explicitly passed
+        self.assertTrue(
+            CommitOperationDelete(
+                path_in_repo="path/to/folder", is_folder=True
+            ).is_folder
+        )
+        self.assertTrue(
+            CommitOperationDelete(
+                path_in_repo="path/to/folder.md", is_folder=True
+            ).is_folder
+        )
+
+    def test_is_folder_wrong_value(self):
+        with self.assertRaises(ValueError):
+            CommitOperationDelete(path_in_repo="path/to/folder", is_folder="any value")
diff --git a/tests/test_file_download.py b/tests/test_file_download.py
index 9ae86fb4..aa029a80 100644
--- a/tests/test_file_download.py
+++ b/tests/test_file_download.py
@@ -397,6 +397,49 @@ class StagingCachedDownloadTest(unittest.TestCase):
                 )
 
 
+@pytest.mark.usefixtures("fx_cache_dir")
+class StagingCachedDownloadOnAwfulFilenamesTest(unittest.TestCase):
+    """Implement regression tests for #1161.
+
+    Issue was on filename not url encoded by `hf_hub_download` and `hf_hub_url`.
+
+    See https://github.com/huggingface/huggingface_hub/issues/1161
+    """
+
+    cache_dir: Path
+    repo_id = "valid_org/repo_with_awful_filename"
+    subfolder = "subfolder/to?"
+    filename = "awful?filename%you:should,never.give"
+    filepath = "subfolder/to?/awful?filename%you:should,never.give"
+    expected_url = "https://hub-ci.huggingface.co/valid_org/repo_with_awful_filename/resolve/main/subfolder/to%3F/awful%3Ffilename%25you%3Ashould%2Cnever.give"
+
+    def test_hf_hub_url_on_awful_filepath(self):
+        self.assertEqual(hf_hub_url(self.repo_id, self.filepath), self.expected_url)
+
+    def test_hf_hub_url_on_awful_subfolder_and_filename(self):
+        self.assertEqual(
+            hf_hub_url(self.repo_id, self.filename, subfolder=self.subfolder),
+            self.expected_url,
+        )
+
+    def test_hf_hub_download_on_awful_filepath(self):
+        local_path = hf_hub_download(
+            self.repo_id, self.filepath, cache_dir=self.cache_dir
+        )
+        # Local path is not url-encoded
+        self.assertTrue(local_path.endswith(self.filepath))
+
+    def test_hf_hub_download_on_awful_subfolder_and_filename(self):
+        local_path = hf_hub_download(
+            self.repo_id,
+            self.filename,
+            subfolder=self.subfolder,
+            cache_dir=self.cache_dir,
+        )
+        # Local path is not url-encoded
+        self.assertTrue(local_path.endswith(self.filepath))
+
+
 class CreateSymlinkTest(unittest.TestCase):
     @patch("huggingface_hub.file_download.are_symlinks_supported")
     def test_create_relative_symlink_concurrent_access(
diff --git a/tests/test_hf_api.py b/tests/test_hf_api.py
index 831a985a..cdb53cfa 100644
--- a/tests/test_hf_api.py
+++ b/tests/test_hf_api.py
@@ -59,6 +59,7 @@ from huggingface_hub.hf_api import (
     repo_type_and_id_from_hf_id,
 )
 from huggingface_hub.utils import (
+    EntryNotFoundError,
     HfFolder,
     HfHubHTTPError,
     RepositoryNotFoundError,
@@ -1024,6 +1025,60 @@ class CommitApiTest(HfApiCommonTestWithLogin):
             self._api.delete_repo(repo_id=REPO_NAME)
 
 
+class HfApiDeleteFolderTest(HfApiCommonTestWithLogin):
+    def setUp(self):
+        self.repo_id = f"{USER}/{repo_name('create_commit_delete_folder')}"
+        self._api.create_repo(repo_id=self.repo_id, exist_ok=False)
+
+        self._api.create_commit(
+            repo_id=self.repo_id,
+            commit_message="Init repo",
+            operations=[
+                CommitOperationAdd(path_or_fileobj=b"data", path_in_repo="1/file_1.md"),
+                CommitOperationAdd(path_or_fileobj=b"data", path_in_repo="1/file_2.md"),
+                CommitOperationAdd(path_or_fileobj=b"data", path_in_repo="2/file_3.md"),
+            ],
+        )
+
+    def tearDown(self):
+        self._api.delete_repo(repo_id=self.repo_id)
+
+    @retry_endpoint
+    def test_create_commit_delete_folder_implicit(self):
+        self._api.create_commit(
+            operations=[CommitOperationDelete(path_in_repo="1/")],
+            commit_message="Test delete folder implicit",
+            repo_id=self.repo_id,
+        )
+
+        with self.assertRaises(EntryNotFoundError):
+            hf_hub_download(self.repo_id, "1/file_1.md", use_auth_token=self._token)
+
+        with self.assertRaises(EntryNotFoundError):
+            hf_hub_download(self.repo_id, "1/file_2.md", use_auth_token=self._token)
+
+        # Still exists
+        hf_hub_download(self.repo_id, "2/file_3.md", use_auth_token=self._token)
+
+    @retry_endpoint
+    def test_create_commit_delete_folder_explicit(self):
+        self._api.delete_folder(path_in_repo="1", repo_id=self.repo_id)
+        with self.assertRaises(EntryNotFoundError):
+            hf_hub_download(self.repo_id, "1/file_1.md", use_auth_token=self._token)
+
+    @retry_endpoint
+    def test_create_commit_failing_implicit_delete_folder(self):
+        with self.assertRaisesRegex(
+            EntryNotFoundError,
+            "Make sure to differentiate file and folder paths",
+        ):
+            self._api.create_commit(
+                operations=[CommitOperationDelete(path_in_repo="1")],
+                commit_message="Failing delete folder",
+                repo_id=self.repo_id,
+            )
+
+
 class HfApiTagEndpointTest(HfApiCommonTestWithLogin):
     _user = USER
     _repo_id: str