Push to hub mixins that do not leverage git (#847)

* Propose a non-git mixin

* Fix merge

* Improvements non git mixin (#618)

Add in commit_message + tests
Co-authored-by: Lysandre <[email protected]>

* Big refactor

* FIX typos in contributing.md

* Remove redefined logger in HfApi.py

* Use upload_folder in both mixins + some docstring

* moved back logger to top of hf_api ><

* space in documentation can be ambiguous

* WIP started deprecation

* deprecate skip lfs file and use Path

* added decorator to deprecate specific arguments + unittests for it

* simplified tests

* proper decorators

* fix docstring

* hubmixin: fixed existing tests + add http one

* unique repo names across tests

* make push_to_hub_keras work + tests

* logs are not overwritten in push_to_hub_keras

* flake8

* refacto push_to_hub from mixin.save_pretrained

* deprecate positional argument in version 0.12

* remove docstring for deprecated skip_lfs_files

* delete old logs when uploading keras model to hub

* remove TODO in tests

* remove useless todo

* Update src/huggingface_hub/hub_mixin.py

* flake8

* remove un-explicit _generate_url helper

* exclude some folders from flake8

* clean pr

* move out tests fixing to other issue

* introduce decorator to deprecate tests

* docstring

* fix pattern in expect_deprecation decorator

* docstring

* remove extras['ml'] integration

* Apply suggestions from code review

Co-authored-by: Omar Sanseviero <[email protected]>

* use expect_deprecation decorator in test

* remove caret in comment

* explicit mocked model

* delete unused and failing tett

* optional path_in_repo for root

* revert back token docstring

* Update src/huggingface_hub/hf_api.py

Co-authored-by: Lysandre Debut <[email protected]>

Co-authored-by: Zachary Mueller <[email protected]>
Co-authored-by: Wauplin <[email protected]>
Co-authored-by: Omar Sanseviero <[email protected]>

Author: 4 people

.gitignore

@@ -109,6 +109,7 @@ venv/
 ENV/
 env.bak/
 venv.bak/
+.venv*
 
 # Spyder project settings
 .spyderproject

CONTRIBUTING.md

@@ -258,13 +258,14 @@ repository they can be run with the following:
 
 ```bash
 $ HUGGINGFACE_CO_STAGING=1 python -m pytest -sv ./tests
+```
 
-In fact, that's how `make test` is implemented (sans the `pip install` line)!
+In fact, that's how `make test` is implemented (without the `pip install` line)!
 
 You can specify a smaller set of tests in order to test only the feature
 you're working on.
 
-For example, the following will only run the tests hel in the `test_repository.py` file:
+For example, the following will only run the tests in the `test_repository.py` file:
 
 ```bash
 $ HUGGINGFACE_CO_STAGING=1 python -m pytest -sv ./tests/test_repository.py

docs/source/how-to-manage.mdx

@@ -1,6 +1,6 @@
 # Create and manage a repository
 
-A repository is a space for you to store your model or dataset files. This guide will show you how to:
+A repository is a place where you can store your model or dataset files. This guide will show you how to:
 
 * Create and delete a repository.
 * Adjust repository visibility.

setup.cfg

@@ -48,6 +48,7 @@ multi_line_output = 3
 use_parentheses = True
 
 [flake8]
+exclude = .git,__pycache__,old,build,dist,.venv*
 ignore = E203, E501, E741, W503
 max-line-length = 88
 

src/huggingface_hub/hf_api.py

@@ -73,9 +73,8 @@
 else:
     from typing_extensions import Literal, TypedDict
 
-
-REGEX_DISCUSSION_URL = re.compile(r".*/discussions/(\d+)$")
 USERNAME_PLACEHOLDER = "hf_user"
+_REGEX_DISCUSSION_URL = re.compile(r".*/discussions/(\d+)$")
 
 logger = logging.get_logger(__name__)
 
@@ -1519,7 +1518,7 @@ def list_repo_files(
         )
         return [f.rfilename for f in repo_info.siblings]
 
-    @_deprecate_positional_args
+    @_deprecate_positional_args(version="0.12")
     def create_repo(
         self,
         repo_id: str = None,
@@ -2006,6 +2005,11 @@ def create_commit(
                 " `CommitOperationDelete`"
             )
 
+        logger.debug(
+            f"About to commit to the hub: {len(additions)} addition(s) and"
+            f" {len(deletions)} deletion(s)."
+        )
+
         for addition in additions:
             addition.validate()
 
@@ -2189,20 +2193,14 @@ def upload_file(
             create_pr=create_pr,
             parent_commit=parent_commit,
         )
-        if pr_url is not None:
-            re_match = re.match(REGEX_DISCUSSION_URL, pr_url)
-            if re_match is None:
-                raise RuntimeError(
-                    "Unexpected response from the hub, expected a Pull Request URL but"
-                    f" got: '{pr_url}'"
-                )
-            revision = quote(f"refs/pr/{re_match[1]}", safe="")
 
+        if pr_url is not None:
+            revision = quote(_parse_revision_from_pr_url(pr_url), safe="")
         if repo_type in REPO_TYPES_URL_PREFIXES:
             repo_id = REPO_TYPES_URL_PREFIXES[repo_type] + repo_id
         revision = revision if revision is not None else DEFAULT_REVISION
+        # Similar to `hf_hub_url` but it's "blob" instead of "resolve"
         return f"{self.endpoint}/{repo_id}/blob/{revision}/{path_in_repo}"
-        # ^ Similar to `hf_hub_url` but it's "blob" instead of "resolve"
 
     def upload_folder(
         self,
@@ -2313,25 +2311,8 @@ def upload_folder(
             if commit_message is not None
             else f"Upload {path_in_repo} with huggingface_hub"
         )
-        folder_path = os.path.normpath(os.path.expanduser(folder_path))
-        if not os.path.isdir(folder_path):
-            raise ValueError(f"Provided path: '{folder_path}' is not a directory")
-
-        files_to_add: List[CommitOperationAdd] = []
-        for dirpath, _, filenames in os.walk(folder_path):
-            for filename in filenames:
-                abs_path = os.path.join(dirpath, filename)
-                rel_path = os.path.relpath(abs_path, folder_path)
-                files_to_add.append(
-                    CommitOperationAdd(
-                        path_or_fileobj=abs_path,
-                        path_in_repo=os.path.normpath(
-                            os.path.join(path_in_repo, rel_path)
-                        ).replace(os.sep, "/"),
-                    )
-                )
 
-        logger.debug(f"About to upload / commit {len(files_to_add)} files to the Hub")
+        files_to_add = _prepare_upload_folder_commit(folder_path, path_in_repo)
 
         pr_url = self.create_commit(
             repo_type=repo_type,
@@ -2346,20 +2327,12 @@ def upload_folder(
         )
 
         if pr_url is not None:
-            re_match = re.match(REGEX_DISCUSSION_URL, pr_url)
-            if re_match is None:
-                raise RuntimeError(
-                    "Unexpected response from the hub, expected a Pull Request URL but"
-                    f" got: '{pr_url}'"
-                )
-            revision = quote(f"refs/pr/{re_match[1]}", safe="")
-
+            revision = quote(_parse_revision_from_pr_url(pr_url), safe="")
         if repo_type in REPO_TYPES_URL_PREFIXES:
             repo_id = REPO_TYPES_URL_PREFIXES[repo_type] + repo_id
-
         revision = revision if revision is not None else DEFAULT_REVISION
+        # Similar to `hf_hub_url` but it's "tree" instead of "resolve"
         return f"{self.endpoint}/{repo_id}/tree/{revision}/{path_in_repo}"
-        # ^ Similar to `hf_hub_url` but it's "tree" instead of "resolve"
 
     def delete_file(
         self,
@@ -3231,8 +3204,8 @@ def get_token(cls) -> Optional[str]:
         """
         Get token or None if not existent.
 
-        Note that a token can be also provided using the `HUGGING_FACE_HUB_TOKEN`
-        environment variable.
+        Note that a token can be also provided using the
+        `HUGGING_FACE_HUB_TOKEN` environment variable.
 
         Returns:
             `str` or `None`: The token, `None` if it doesn't exist.
@@ -3258,6 +3231,48 @@ def delete_token(cls):
             pass
 
 
+def _prepare_upload_folder_commit(
+    folder_path: str, path_in_repo: str
+) -> List[CommitOperationAdd]:
+    """Generate the list of Add operations for a commit to upload a folder."""
+    folder_path = os.path.normpath(os.path.expanduser(folder_path))
+    if not os.path.isdir(folder_path):
+        raise ValueError(f"Provided path: '{folder_path}' is not a directory")
+
+    files_to_add: List[CommitOperationAdd] = []
+    for dirpath, _, filenames in os.walk(folder_path):
+        for filename in filenames:
+            abs_path = os.path.join(dirpath, filename)
+            rel_path = os.path.relpath(abs_path, folder_path)
+            files_to_add.append(
+                CommitOperationAdd(
+                    path_or_fileobj=abs_path,
+                    path_in_repo=os.path.normpath(
+                        os.path.join(path_in_repo, rel_path)
+                    ).replace(os.sep, "/"),
+                )
+            )
+    return files_to_add
+
+
+def _parse_revision_from_pr_url(pr_url: str) -> str:
+    """Safely parse revision number from a PR url.
+
+    Example:
+    ```py
+    >>> _parse_revision_from_pr_url("https://huggingface.co/bigscience/bloom/discussions/2")
+    "refs/pr/2"
+    ```
+    """
+    re_match = re.match(_REGEX_DISCUSSION_URL, pr_url)
+    if re_match is None:
+        raise RuntimeError(
+            "Unexpected response from the hub, expected a Pull Request URL but"
+            f" got: '{pr_url}'"
+        )
+    return f"refs/pr/{re_match[1]}"
+
+
 api = HfApi()
 
 set_access_token = api.set_access_token
@@ -3301,3 +3316,5 @@ def delete_token(cls):
 edit_discussion_comment = api.edit_discussion_comment
 rename_discussion = api.rename_discussion
 merge_pull_request = api.merge_pull_request
+
+_validate_or_retrieve_token = api._validate_or_retrieve_token