Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Dataset card template overhaul #1708

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Dataset card template overhaul #1708

merged 6 commits into from
Oct 10, 2023

Conversation

mariosasko
Copy link
Contributor

@mariosasko mariosasko commented Oct 3, 2023 •
edited
Loading

It seems to me that new model releases are more likely to have a filled-out (or fewer blank and "out-of-template" sections/fields) repo card than new dataset releases.
So, this PR introduces the following changes to the dataset card template to make working with it less cumbersome and improve the situation (hopefully):

  • Aligns the structure and section names with the model card template. Besides consistency, this should also mean less time is needed to write the repo cards for joint model-dataset releases
  • Adds (markdown) comments to the sections to make it clear what information a particular section expects
  • Removes unnecessary/outdated sections (e.g., the viewer displays data samples, value statistics/ranges, split sizes, and more, so we can simplify the Dataset Structure section) and fields (e.g., homepage, leaderboard, etc.)

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 3, 2023 •
edited
Loading

The documentation is not available anymore as the PR was closed or merged.

@codecov
Copy link

codecov bot commented Oct 3, 2023 •
edited
Loading

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (90c1182) 82.30% compared to head (af900cc) 82.42%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1708      +/-   ##
==========================================
+ Coverage   82.30%   82.42%   +0.12%     
==========================================
  Files          62       62              
  Lines        7226     7226              
==========================================
+ Hits         5947     5956       +9     
+ Misses       1279     1270       -9

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

julien-c
julien-c reviewed Oct 4, 2023
View reviewed changes
src/huggingface_hub/templates/modelcard_template.md
@@ -77,7 +77,7 @@ Use the code below to get started with the model.

### Training Data

<!-- This should link to a Data Card, perhaps with a short stub of information on what the training data is all about as well as documentation related to data pre-processing or additional filtering. -->
Copy link
Member

Choose a reason for hiding this comment

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

👍

julien-c
julien-c reviewed Oct 4, 2023
View reviewed changes
Copy link
Member

@julien-c julien-c left a comment

Choose a reason for hiding this comment

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

looks good to me from a super quick glance

I assume you have checked it still works in DatasetCard.from_template?

cc @EziOzoani @meg-huggingface too on dataset documentation. cc @yjernite too

Wauplin
Wauplin reviewed Oct 4, 2023
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks for opening the PR @mariosasko! Looks good to me as well but from a non-expert point of view.

I posted a message in slack (private) as well. To ease the review, here are the actual files (I found it easier to review than the diff):

@mariosasko
Copy link
Contributor Author

@julien-c

I assume you have checked it still works in DatasetCard.from_template?

We have tests in tests/test_repocard.py that check this :)

meg-huggingface
meg-huggingface requested changes Oct 4, 2023
View reviewed changes
Copy link
Contributor

@meg-huggingface meg-huggingface left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great, thank you for this!!
Adding some minor changes in wording/spelling.

src/huggingface_hub/templates/datasetcard_template.md Show resolved Hide resolved
src/huggingface_hub/templates/datasetcard_template.md Outdated Show resolved Hide resolved
src/huggingface_hub/templates/datasetcard_template.md Outdated Show resolved Hide resolved
src/huggingface_hub/templates/datasetcard_template.md Outdated Show resolved Hide resolved
mariosasko and others added 3 commits October 4, 2023 21:34
@mariosasko @meg-huggingface
Apply suggestions from code review
131b513 
Co-authored-by: meg <90473723+meg-huggingface@users.noreply.github.com>
@mariosasko
Address more comments
51a3bb1
@mariosasko
Resolve conflict
f9f9b2b
@mariosasko
Copy link
Contributor Author

Ready for the final review :)!

(CI failures are unrelated to the changes)

meg-huggingface
meg-huggingface approved these changes Oct 6, 2023
View reviewed changes
Copy link
Contributor

@meg-huggingface meg-huggingface left a comment

Choose a reason for hiding this comment

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

So happy for this, thanks for the work!

@Wauplin
Copy link
Contributor

Wauplin commented Oct 10, 2023

Great, I'm merging it and will release it this week :)

@Wauplin Wauplin merged commit 6dd7ee8 into main Oct 10, 2023
16 checks passed
@Wauplin Wauplin deleted the update-datasetcard-template branch October 10, 2023 09:53
Wauplin added a commit that referenced this pull request Oct 10, 2023
@mariosasko @meg-huggingface @Wauplin
Dataset card template overhaul (#1708)
147702b 
* Dataset card template overhaul

* Add Privacy Considerations subsection

* Apply suggestions from code review

Co-authored-by: meg <90473723+meg-huggingface@users.noreply.github.com>

* Address more comments

---------

Co-authored-by: meg <90473723+meg-huggingface@users.noreply.github.com>
Co-authored-by: Lucain <lucainp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wauplin Wauplin Wauplin left review comments

@julien-c julien-c julien-c left review comments

@meg-huggingface meg-huggingface meg-huggingface approved these changes

@lhoestq lhoestq Awaiting requested review from lhoestq

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mariosasko @HuggingFaceDocBuilderDev @Wauplin @julien-c @lhoestq @meg-huggingface