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.

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

Update Hub and huggingface_hub docs #293

Merged
merged 3 commits into from
Aug 24, 2021
Merged

Update Hub and huggingface_hub docs #293

merged 3 commits into from
Aug 24, 2021

Conversation

osanseviero
Copy link
Contributor

This PR does two things

  • Updates the add a model guide based on feedback
  • Does a general cleanup/update/simplification of huggingface_hub README.

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.

good improvements

(nit) Not a fan of the 80-char line wrapping as it makes subsequent diffs hard to read

@LysandreJik
Copy link
Member

Was actually thinking of bumping up the black character limit to 119 like transformers, it greatly improves readability imo

@julien-c
Copy link
Member

hmm those are .md files

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, thanks for completing the docs with the docstrings!

@LysandreJik
Copy link
Member

hmm those are .md files

Yes, I was talking about the Python files, not the files here. Bouncing back on your comment that 80 characters isn't necessarily more readable!

@osanseviero
Copy link
Contributor Author

I wrapped all the file since I realize a section of the file was wrapped and the other was not. The diff is easy to read in rich mode for markdown though, and for future PRs it should be easy if we stay that way.

@osanseviero osanseviero merged commit 880e3dc into main Aug 24, 2021
@osanseviero osanseviero deleted the new_docs branch August 24, 2021 14:13
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.

3 participants