MNT deprecate name and organization in favor of repo_id

[adrinjalali]

Fixes #706

Tests and deprecations on other methods to be added if we're okay with the approach.

cc @LysandreJik @osanseviero

[osanseviero]

I like this approach for deprecating! I'm all in favour of updating the other params as well.

Thank you very much for this PR! 🚀 🔥

[osanseviero]

token should always be a string, so I would keep that. If not specified, it retrieves the stored token.

[adrinjalali]

_validate_or_retrieve_token seems to disagree with you I think. Although in this case you're right I guess? I'm not sure if we're being consistent in treatment of token and use_auth_token across this file.

[adrinjalali]

This is now ready for another review!

[osanseviero]

This looks good to me!

[adrinjalali]

I'm not sure if the failed CI is related to this PR.

[LysandreJik]

Thanks a lot for solving the issue!

You're solving the issue by adding a new argument repo_id, which is fine, but I think it's misleading in some cases, as I would expect the name to work correctly as a positional argument. I personally would use the method as follows:

from huggingface_hub import create_repo

create_repo("my-brand-new-repo")
create_repo("my-brand-new-repo", organization="huggingface")

However with this approach this results in a warning that may be a bit arcane:

>>> create_repo('<repo>')

/.../huggingface_hub/src/huggingface_hub/hf_api.py:65: FutureWarning: `name` and `organization` input arguments are deprecated and will be removed in v0.7. Pass `repo_id` instead.
  FutureWarning,
'https://huggingface.co/<user>/<repo>'

I don't really know what's name and why I'm being prompted to change it here, given I didn't pass it with the keyword.

Furthermore, it makes the following API usage deprecated, even though it is both documented here and feels like the API we're trying to push forward.

>>> create_repo('<org>/<repo>')

/.../huggingface_hub/src/huggingface_hub/hf_api.py:65: FutureWarning: `name` and `organization` input arguments are deprecated and will be removed in v0.7. Pass `repo_id` instead.
  FutureWarning,
'https://huggingface.co/<org>/<repo>'

---

I would prefer doing the following:

1. Deprecating the organization argument
2. Replacing the name argument by repo_id
3. Adding name as an additional kwarg at the bottom, instead of the repo_id you've added.

This should enable the following usage:

• create_repo("<repo>")
• create_repo("<org>/<repo>")
• create_repo(repo_id="<repo>")
• create_repo(repo_id="<org>/<repo>")
• create_repo("<name>", organization="<org>") # Deprecation warning!
• create_repo(name="<name>", organization="<org>") # Deprecation warning!

What do you think?

[adrinjalali]

Test failing with

E           requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://moon-staging.huggingface.co/api/repos/create - You don't have the rights to create a model under this namespace

on:

    def test_push_to_hub(self):
        REPO_NAME = repo_name("PUSH_TO_HUB")
        model = DummyModel()
        model.push_to_hub(
            repo_path_or_name=f"{WORKING_REPO_DIR}/{REPO_NAME}",
            api_endpoint=ENDPOINT_STAGING,
            use_auth_token=self._token,
            git_user="ci",
            git_email="ci@dummy.com",
            config={"num": 7, "act": "gelu_fast"},
        )

I'm not sure how to fix this. @julien-c seems to have written the test.

[LysandreJik]

This looks good! Only left a comment regarding a switch between the org and name, it should solve the CI issue.

[LysandreJik]

I would add a check to ensure that at least repo_id or name is passed, since they are now both optional arguments:

>>> from huggingface_hub import create_repo
>>> create_repo()
Traceback (most recent call last):
  File "/usr/lib/python3.6/code.py", line 91, in runcode
    exec(code, self.locals)
  File "<input>", line 1, in <module>
  File "/home/lysandre/Workspaces/Python/huggingface_hub/src/huggingface_hub/hf_api.py", line 1056, in create_repo
    name, organization = _validate_repo_id_deprecation(repo_id, name, organization)
  File "/home/lysandre/Workspaces/Python/huggingface_hub/src/huggingface_hub/hf_api.py", line 68, in _validate_repo_id_deprecation
    if "/" in repo_id:
TypeError: argument of type 'NoneType' is not iterable

Something like the following

Suggested change

	name, organization = _validate_repo_id_deprecation(repo_id, name, organization)
	if name is None and repo_id is None:
	raise ValueError("Please pass either `name` or `repo_id`")
	name, organization = _validate_repo_id_deprecation(repo_id, name, organization)

[osanseviero]

+1, but since name is deprecated, I would just ask to specify repo_id.

[LysandreJik]

Indeed, good call!

[adrinjalali]

@LysandreJik if you're happy with the changes, please feel free to merge.

[LysandreJik]

LGTM, great work!

[LysandreJik]

Resolved conflicts, will merge when tests pass

[lhoestq]

would love to be pinged if you do changes in HfApi.create_repo or HfApi.upload_file in the future !