-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Huggingface model transfer class with save_pretrained in download #158
Added Huggingface model transfer class with save_pretrained in download #158
Conversation
be1b5c7
to
c4f22e0
Compare
…with_save_pretrained' into feature/#143_Add_model_transfer_with_save_pretrained
39bce56
to
cdcccc7
Compare
…with_save_pretrained' into feature/#143_Add_model_transfer_with_save_pretrained
exasol_transformers_extension/utils/huggingface_hub_bucketfs_model_transfer_sp.py
Outdated
Show resolved
Hide resolved
exasol_transformers_extension/utils/huggingface_hub_bucketfs_model_transfer_sp.py
Outdated
Show resolved
Hide resolved
exasol_transformers_extension/utils/huggingface_hub_bucketfs_model_transfer_sp.py
Outdated
Show resolved
Hide resolved
|
||
|
||
@runtime_checkable | ||
class ModelFactoryProtocol(Protocol): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that got duplicated, extract from both files and use the same
exasol_transformers_extension/utils/huggingface_hub_bucketfs_model_transfer_sp.py
Outdated
Show resolved
Hide resolved
self.bucketfs_location_mock.reset_mock() | ||
self.temporary_directory_factory_mock.reset_mock() | ||
self.model_factory_mock.reset_mock() | ||
self.bucketfs_model_uploader_mock.reset_mock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.bucketfs_model_uploader_factory_mock is missing
def test_download_function_call(): | ||
test_setup = TestSetup() | ||
test_setup.downloader.download_from_huggingface_hub(model_factory=test_setup.model_factory_mock) | ||
cache_dir = test_setup.temporary_directory_factory_mock.create().__enter__() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cache_dir = test_setup.temporary_directory_factory_mock.create().__enter__() | |
cache_dir = mock_cast(test_setup.temporary_directory_factory_mock.create().__enter__).return_value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise you are adding a call to the mocks mock_calls
test_setup = TestSetup() | ||
test_setup.downloader.download_from_huggingface_hub(model_factory=test_setup.model_factory_mock) | ||
cache_dir = test_setup.temporary_directory_factory_mock.create().__enter__() | ||
model_save_path = (test_setup.downloader._tmpdir_name/"pretrained"/test_setup.model_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spaces in path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should not access private attributes, why do you need it?
tests/unit_tests/utils/test_huggingface_hub_bucketfs_model_transfer_sp.py
Show resolved
Hide resolved
tests/unit_tests/utils/test_huggingface_hub_bucketfs_model_transfer_sp.py
Outdated
Show resolved
Hide resolved
Team member currently not available, all findings are fixed.
All Submissions:
[CodeBuild]
to the commit messageFixes #143