Skip to content

Conversation

@osanseviero
Copy link
Contributor

Fix #100

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Nov 17, 2023

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

@coyotte508
Copy link
Member

Will be useful for queries like this: https://huggingface.co/migtissera/HelixNet/discussions/9

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.

left a few comments

@julien-c julien-c requested a review from Wauplin November 17, 2023 17:23
Copy link
Member

@davanstrien davanstrien left a comment

Choose a reason for hiding this comment

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

A few small suggestions (feel free to ignore!)

@Wauplin
Copy link
Contributor

Wauplin commented Nov 17, 2023

(can we wait Monday before merging even if already approved? Just want to read it without rushing and won't have time today :) )

osanseviero and others added 3 commits November 17, 2023 19:13
Co-authored-by: Daniel van Strien <davanstrien@users.noreply.github.com>
Co-authored-by: Julien Chaumond <julien@huggingface.co>
@julien-c
Copy link
Member

yes for sure let's wait a few days – also interested in @LysandreJik's read

@osanseviero osanseviero requested a review from julien-c November 17, 2023 18:46
Copy link
Member

@LysandreJik LysandreJik 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 in principle, but I'd add one or two lines to specify explicitly that we're not doing anything funky on the user's side. We're absolutely optimizing for minimal number of requests and we're not sending any information. Everything happens server-side.

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.

Looks good!

@osanseviero
Copy link
Contributor Author

Thanks everyone for the reviews. I'll merge this later today if nobody else adds comments

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Looks great!

A small clarification regarding model downloads would be to define a "download" as every use of the target file, including when it's already cached in the user's computer. Probably not necessary.

@osanseviero
Copy link
Contributor Author

For now let's go with this and later we can iterate

@osanseviero osanseviero reopened this Nov 20, 2023
@osanseviero osanseviero merged commit 9662454 into main Nov 20, 2023
@osanseviero osanseviero deleted the download-docs branch November 20, 2023 21:59
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

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.

How are download stats generated for datasets on the hub

9 participants