-
Notifications
You must be signed in to change notification settings - Fork 236
[ENH] Add loaders for Monster datasets with hugging_face dependency #3141
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for contributing to
|
|
@baraline pls have a look. |
|
At first glance the file looks good. I think you can remove the univariate dataset name list, as you pull all the dataset names already. Concerning the tests, you can look at existing tests file from other loaders, for exemple https://github.com/aeon-toolkit/aeon/blob/main/aeon/datasets/tests/test_data_loaders.py Also, you have some pre-commit failure, you can refer to the developer documentation it explains how to install it, it will correct most of the issues by itself when you try to commit and signal you the others. |
|
@baraline how do i resolve |
|
The check soft dep should stay, it is used to check if the dependency is installed (as it is optional) and raide appropriate messages to the users. Don't have access to my computer rn, so add the check soft dep back and i'll check the errors throwed when I can later |
baraline
left a comment
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.
Test are failing because the test skip is wrongly parameterized. The wrong check is applied to validate if the test should be run or not. You should do
@pytest.mark.skipif(
not _check_soft_dependencies("hugging-face", severity="none"),
reason="required soft dependency hugging-face not available",
)
Instead of
@pytest.mark.skipif(
PR_TESTING,
reason="Only run on overnights because of read from internet.",
)
|
You should test with huggingface-hub (the dependency you added in the project.toml), not with hugging-face. You correctly did it when you created the loader like this : _check_soft_dependencies("huggingface-hub", severity="none") My example didn't take the correct name. |
my bad!! |
|
The remaining failure is not linked to what you did, so we should be good to go. We'll just wait for it to be resolved. Thanks ! |
thank you for guiding me!! |
Reference Issues/PRs
Fixes #2985
What does this implement/fix? Explain your changes.
Implements a new file for loading Monster datasets from
hugginngface_hub.Creates methods for loading list of available monster datasets in huggingface_hub and downloading the datasets.
Does your contribution introduce a new dependency? If yes, which one?
hugging_faceAny other comments?
I am yet to implement the
test_monster_loadersfile.PR checklist
For all contributions
For new estimators and functions
__maintainer__at the top of relevant files and want to be contacted regarding its maintenance. Unmaintained files may be removed. This is for the full file, and you should not add yourself if you are just making minor changes or do not want to help maintain its contents.For developers with write access