Skip to content

Conversation

@binh234
Copy link
Contributor

@binh234 binh234 commented Aug 10, 2021

No description provided.

Copy link
Member

@albertvillanova albertvillanova left a comment

Choose a reason for hiding this comment

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

Hi @binh234, thanks for this dataset!

There are only some few issues to be fixed before we can merge it to master:

  • One is about style. Please run make style (at the root directory of the datasets repo) to fix it automatically.
  • The other one is that it misses the dataset card (the README.md file). In order to be able to find this dataset when searching among all datasets in our Datasets Hub, we need all the dataset tags and metadata. In order to generate it, please have a look at

# dl_manager is a datasets.download.DownloadManager that can be used to download and extract URLs
# It can accept any type or nested list/dict and will give back the same structure with the url replaced with path to local files.
# By default the archives will be extracted and a path to a cached folder where they are extracted is returned instead of the archive
dl_path = dl_manager.download_and_extract(_DATA_URL.format(self.config.name))
Copy link
Member

Choose a reason for hiding this comment

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

I think there is no need of the .format(self.config.name) part of the code.

Copy link
Member

@albertvillanova albertvillanova left a comment

Choose a reason for hiding this comment

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

Thanks again for all your fixes and improvements! :)

Just some minor fixes and we are ready to merge into master.

Copy link
Member

@albertvillanova albertvillanova left a comment

Choose a reason for hiding this comment

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

Thank you! You did a great job! :)

@albertvillanova albertvillanova merged commit 0667481 into huggingface:master Aug 12, 2021
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