-
Notifications
You must be signed in to change notification settings - Fork 3.1k
More consistent naming #2611
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
More consistent naming #2611
Conversation
stas00
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.
Thank you for doing this polish, @mariosasko! Great work.
I left a few small remarks in the changes.
README.md
Outdated
| </p> | ||
|
|
||
| `🤗Datasets` is a lightweight library providing **two** main features: | ||
| `🤗 Datasets` is a lightweight library providing **two** main features: |
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.
IMHO, here and everywhere else in this doc needs to be either datasets or 🤗 Datasets - no backticks!
backticks are for the module name, which is not 🤗 Datasets but datasets
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.
since there is only one python package called datasets if you go with the former, you probably don't need the emoji.
I'd say to be consistent with transformers docs, most of the time ideally it should be: 🤗 Datasets (as in 🤗 Transformers).
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.
Unless you are writing:
Main differences between
🤗 Datasetsandtfds
a few paras below, which looks very unbalanced.
Here you probably want to go for one of:
datasetsandtfds- 🤗 Datasets and TensorFlow
tfds
Just a suggestion of course...
docs/source/index.rst
Outdated
| 🤗 Datasets currently provides access to ~100 NLP datasets and ~10 evaluation metrics and is designed to let the community easily add and share new datasets and evaluation metrics. You can browse the full set of datasets with the live 🤗 Datasets viewer. | ||
|
|
||
| 🤗Datasets originated from a fork of the awesome TensorFlow Datasets and the HuggingFace team want to deeply thank the TensorFlow Datasets team for building this amazing library. More details on the differences between 🤗Datasets and tfds can be found in the section Main differences between 🤗Datasets and tfds. | ||
| 🤗 Datasets originated from a fork of the awesome TensorFlow Datasets and the HuggingFace team want to deeply thank the TensorFlow Datasets team for building this amazing library. More details on the differences between 🤗 Datasets and tfds can be found in the section Main differences between 🤗 Datasets and tfds. |
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.
| 🤗 Datasets originated from a fork of the awesome TensorFlow Datasets and the HuggingFace team want to deeply thank the TensorFlow Datasets team for building this amazing library. More details on the differences between 🤗 Datasets and tfds can be found in the section Main differences between 🤗 Datasets and tfds. | |
| 🤗 Datasets originated from a fork of the awesome TensorFlow Datasets and the HuggingFace team want to deeply thank the TensorFlow Datasets team for building this amazing library. More details on the differences between 🤗 Datasets and tfds can be found in the section Main differences between 🤗 Datasets and `tfds`. |
but see my earlier 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.
I think it's fine this way. We could also change it to "Tensorflow Datasets"
Co-authored-by: Stas Bekman <[email protected]>
Co-authored-by: Stas Bekman <[email protected]>
docs/source/share_dataset.rst
Outdated
| ----------------------------------------- | ||
|
|
||
| In this page, we will show you how to share a dataset with the community on the `datasets hub <https://huggingface.co/datasets>`__. | ||
| In this page, we will show you how to share a dataset with the community on the `Datasets Hub <https://huggingface.co/datasets>`__. |
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.
For consistency may want to add 🤗 here and in all the instances of Datasets Hub below?
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.
Just added them, thanks !
lhoestq
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.
Thanks for bringing consistency :) This is much cleaner now
Also feel free to ignore the CI errors, it's red just because some tags are missing in some datasets for which the docstring changed a bit
As per @stas00's suggestion in #2500, this PR inserts a space between the logo and the lib name (
🤗Datasets->🤗 Datasets) for consistency with the Transformers lib. Additionally, more consistent names are used for Datasets Hub, etc.