Improve Mixin #34
Improve Mixin #34
Conversation
setup.py
Outdated
| extras["testing"] = [ | ||
| "pytest", | ||
| ] | ||
| ] + extras["torch"] |
There was a problem hiding this comment.
is there value in having a second CI setup with no torch? (to make sure we don't introduce a dependency on torch inadvertently down the road)
| if isinstance(use_auth_token, str): | ||
| huggingface_token = use_auth_token | ||
| elif use_auth_token is None and repo_url is not None: | ||
| # If the repo url exists, then no need for a token | ||
| huggingface_token = None | ||
| else: | ||
| huggingface_token = HfFolder.get_token() |
There was a problem hiding this comment.
| if isinstance(use_auth_token, str): | |
| huggingface_token = use_auth_token | |
| elif use_auth_token is None and repo_url is not None: | |
| # If the repo url exists, then no need for a token | |
| huggingface_token = None | |
| else: | |
| huggingface_token = HfFolder.get_token() | |
| if isinstance(use_auth_token, str): | |
| huggingface_token = use_auth_token | |
| elif use_auth_token: | |
| huggingface_token = HfFolder.get_token() | |
| else: | |
| huggingface_token = None |
let's use the same logic as elsewhere no?
There was a problem hiding this comment.
The issue with using the same logic as elsewhere is that the API becomes weird to use, in order to cover a rare use-case. Namely, the following will never work:
model.push_to_hub("xxx")it needs to be the following:
model.push_to_hub("xxx", use_auth_token=True)I fail to see when a user would want to push to the hub without having an auth_token, as it's necessary to create a repo. If the repo already exists and one wants to push to it, then the user already has to specify the repo_url to push to.
I think the API is cleaner by having model.push_to_hub("xxx") work if you already have an authentication token in your HF folder.
| files = os.listdir(f"{WORKING_REPO_DIR}/{REPO_NAME}") | ||
| self.assertTrue("config.json" in files) | ||
| self.assertTrue("pytorch_model.bin" in files) | ||
| self.assertEqual(len(files), 2) |
| ) | ||
| self.assertTrue(model.config == {"num": 10, "act": "gelu_fast"}) |
There was a problem hiding this comment.
self.assertEqual should work, no?
Improves the Mixin in the hub by adding support for
api_endpoint,use_auth_token, as well asgit_userandgit_email.It updates the tests so that they run in the staging environment. In doing so, I found a bug relative to absolute paths in
from_pretrained, this fixes that at the same time.