Skip to content

Conversation

@yavuzKomecoglu
Copy link
Contributor

  • The source address of the TTC4900 dataset of @savasy has been updated for direct download.
  • Updated readme.

@yavuzKomecoglu
Copy link
Contributor Author

@lhoestq, lütfen bu PR'ı gözden geçirebilir misiniz?

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi ! Thanks for updating the dataset :)
This is really helpful !

I just added a comment about the URL passed to the dl_managed

Could you also remove the dummy_data.zip file that you added ? We don't need to have a new one + it seems that it's not in the correct location anyway.

Finally if you take a look at the CI you will notice that there is an error about the structure of the README.md:


E           -	Section `Dataset Description` is missing subsection: `Supported Tasks and Leaderboards`.
E           -	Section `Considerations for Using the Data` is missing subsection: `Social Impact of Dataset`.
E           -	Section `Considerations for Using the Data` is missing subsection: `Discussion of Biases`.
E           -	`Considerations for Using the Data` has an extra subsection: `Discussion of Social Impact and Biases`.

You can follow the template from here to fix the structure: https://github.com/huggingface/datasets/blob/master/templates/README.md

To make sure the README.md has the right structure, you can run this test command:

pytest "tests/test_dataset_cards.py::test_changed_dataset_card[ttc4900]"

Feel free to ping me if you have any question or if I can help :)

)

urls_to_download = {
"train": os.path.join(_DOWNLOAD_URL, _FILENAME),
Copy link
Member

@lhoestq lhoestq Jul 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.path.join uses "\" instead of "/" on windows

Suggested change
"train": os.path.join(_DOWNLOAD_URL, _FILENAME),
"train": _DOWNLOAD_URL + "/" + _FILENAME,

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ! This looks all good now :)

@lhoestq lhoestq merged commit 440b14d into huggingface:master Jul 30, 2021
@yavuzKomecoglu
Copy link
Contributor Author

Thanks ! This looks all good now :)

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants