datasets doesn't support # in data paths
State: closed | Author: loubnabnl (NONE)
Labels: name: bug

name: good first issue

name: hacktoberfest

## Describe the bug
dataset files with `#` symbol their paths aren't read correctly.

## Steps to reproduce the bug
The data in folder `c#`of this [dataset](https://huggingface.co/datasets/loubnabnl/bigcode_csharp) can't be loaded. While the folder `c_sharp` with the same data is loaded properly
```python
ds = load_dataset('loubnabnl/bigcode_csharp', split="train", data_files=["data/c#/*"])
```
```
FileNotFoundError: Couldn't find file at https://huggingface.co/datasets/loubnabnl/bigcode_csharp/resolve/27a3166cff4bb18e11919cafa6f169c0f57483de/data/c#/data_0003.jsonl
```

## Environment info
<!-- You can run the command `datasets-cli env` and copy-and-paste its output below. -->
- `datasets` version: 2.5.2
- Platform: macOS-12.2.1-arm64-arm-64bit
- Python version: 3.9.13
- PyArrow version: 9.0.0
- Pandas version: 1.4.3

cc @lhoestq 

Comments: `datasets` doesn't seem to urlencode the directory names here

https://github.com/huggingface/datasets/blob/7feeb5648a63b6135a8259dedc3b1e19185ee4c7/src/datasets/utils/file_utils.py#L109-L111

for example we should have
```python
from datasets.utils.file_utils import hf_hub_url

url = hf_hub_url("loubnabnl/bigcode_csharp", "data/c#/data_0003.jsonl")
print(url)
# Currently returns
# https://huggingface.co/datasets/loubnabnl/bigcode_csharp/resolve/main/data/c#/data_0003.jsonl
# while it should be 
# https://huggingface.co/datasets/loubnabnl/bigcode_csharp/resolve/main/data/c%23/data_0003.jsonl
```

I'll work on this :)

@loubnabnl The dataset you linked in the description of the bug does not work and returns a 404. Where can I find the dataset to reproduce the bug?

I think you can create a dataset repository on the Hub with a dummy file containing a `#`

Ah sorry it was private I just made it public, I can also help with this if needed

@lhoestq Should I url encode also repo_id and revision parameters? I'm not sure what are the valid characters there.

Personally, I would be cautious and only url encode the path parameter.

These are possible solutions (assuming `from urllib.parse import quote`):

1) url encode only the path parameter:
```
# src/datasets/utils/file_utils.py
def hf_hub_url(repo_id: str, path: str, revision: Optional[str] = None) -> str:
    revision = revision or config.HUB_DEFAULT_VERSION
    return config.HUB_DATASETS_URL.format(repo_id=repo_id, path=quote(path), revision=revision)
```
2) url encode all parameters:
```
# src/datasets/utils/file_utils.py
def hf_hub_url(repo_id: str, path: str, revision: Optional[str] = None) -> str:
    revision = revision or config.HUB_DEFAULT_VERSION
    return config.HUB_DATASETS_URL.format(repo_id=quote(repo_id), path=quote(path), revision=quote(revision))
```
3) url encode the whole url:
```
# src/datasets/config.py
HUB_DATASETS_PATH = "/datasets/{repo_id}/resolve/{revision}/{path}"
HUB_DATASETS_URL = HF_ENDPOINT + HUB_DATASETS_PATH
```
```
# src/datasets/utils/file_utils.py
def hf_hub_url(repo_id: str, path: str, revision: Optional[str] = None) -> str:
    revision = revision or config.HUB_DEFAULT_VERSION
    return config.HF_ENDPOINT + quote(config.HUB_DATASETS_PATH.format(repo_id=repo_id, path=path, revision=revision))
```

repo_id can only contain alphanumeric characters and _- so it doesn't need to be encoded.

However I agree it's a good idea to also apply `quote` to the revision as well as in 2. !

Should be fixed by https://github.com/huggingface/datasets/issues/5099 - we'll do a release later today