Make HuggingFace Hub Mixin work with newer utilities #473
Conversation
| ) | ||
| if local_path is not None: | ||
| repo_path = local_path | ||
| if not repo_name.startswith(organization): |
There was a problem hiding this comment.
This line throws an exception for me when organization is not set.
| if not repo_name.startswith(organization): | |
| if organization is not None and not repo_name.startswith(organization): |
Maybe we could also think about deprecating the organization parameter as done in Transformers?
There was a problem hiding this comment.
I think deprecating organization to keep the AdapterHub API similar to HuggingFace makes a lot of sense! I just didn't want to change your API as a first time contributor!
There was a problem hiding this comment.
For now, I've added your suggested change and kept everything backwards compatible. If you want to make this a breaking change and deprecate the organization argument, I can make that shift instead!
There was a problem hiding this comment.
I think we can go for the change but keep the organization parameter for backward compatibility with a deprecation warning, similar to HF does it here: https://github.com/huggingface/transformers/blob/d1c9242db2092e9d96b1b18ba9fe27f3518184a5/src/transformers/utils/hub.py#L891-L899
There was a problem hiding this comment.
Sorry - hadn't seen this notification. Just added the warning!
Co-authored-by: calpt <calpt@mail.de>
In the current master branch, this mixin fails since it's using utilities that no longer exist in Transformers itself. I've updated the Adapter HuggingFace Hub mixin to be compliant by following the update PR from the main transformers branch: https://github.com/huggingface/transformers/pull/18366/files