Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

Add docs #13

Closed
wants to merge 13 commits into from
Closed

Add docs #13

wants to merge 13 commits into from

Conversation

mariosasko
Copy link
Contributor

@mariosasko mariosasko commented Feb 17, 2023

Add docs

@HuggingFaceDocBuilderDev

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

Comment on lines +16 to +17
secrets:
token: ${{ secrets.HUGGINGFACE_PUSH }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LysandreJik Can you help me set up this secret?

@mariosasko
Copy link
Contributor Author

@mishig25 Do you know why the docs generated for this PR are blank (building locally works without any issues)?

Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Super cool! 🤩

I left a few comments, mainly about clarifying or specifying things because I'm not super familiar with filesystems, so definitely take this with a grain of salt if you think this is already obvious to your audience.

docs/source/index.mdx Outdated Show resolved Hide resolved
docs/source/index.mdx Outdated Show resolved Hide resolved
docs/source/index.mdx Show resolved Hide resolved
docs/source/index.mdx Outdated Show resolved Hide resolved
docs/source/index.mdx Outdated Show resolved Hide resolved

Note that, unlike the built-in `open`, the mode in `fsspec`'s `open` defaults to binary mode, i.e. "rb". This means you must explicitly set encoding as "r"/"w" in case of text mode.

## Integration
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it'd make more sense to move the Integration section before the Usage section? It might be good for the user to check if they can use a URL with an integration before they start using the filesystem operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This order comes from the s3fs docs, so I think I'll leave it as-is.

docs/source/index.mdx Outdated Show resolved Hide resolved
{'repo_type': 'model', 'repo_id': 'johndoe/cityskapes', 'revision': 'dev', 'path/in/repo': ''}
```

## Authentication
Copy link
Member

Choose a reason for hiding this comment

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

Maybe also move this under Integration so users know beforehand they should be logged in with their Hugging Face account. So:

## Integration
## Authentication
## Usage

docs/source/integration_zoo.mdx Outdated Show resolved Hide resolved
docs/source/integration_zoo.mdx Outdated Show resolved Hide resolved
@mariosasko mariosasko requested a review from stevhliu February 20, 2023 14:51
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Awesome, this looks great!

docs/source/index.mdx Outdated Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants