Discussion on version identifier & MockDataLoaderManager for test data
CONTRIBUTOR
usually you can replace `download` in your dataset script with `download_and_prepare()` - could you share the code for your dataset here? :-) 
2020-05-18T21:00:31Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630432633
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDQzMjYzMw==
https://api.github.com/repos/huggingface/datasets/issues/comments/630432633/reactions
2020-05-18T21:00:31Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630432633
https://avatars.githubusercontent.com/u/23423619?v=4
https://api.github.com/users/patrickvonplaten/events{/privacy}
https://api.github.com/users/patrickvonplaten/followers
https://api.github.com/users/patrickvonplaten/following{/other_user}
https://api.github.com/users/patrickvonplaten/gists{/gist_id}
https://github.com/patrickvonplaten
patrickvonplaten
MDQ6VXNlcjIzNDIzNjE5
https://api.github.com/users/patrickvonplaten/orgs
https://api.github.com/users/patrickvonplaten/received_events
https://api.github.com/users/patrickvonplaten/repos
https://api.github.com/users/patrickvonplaten/starred{/owner}{/repo}
https://api.github.com/users/patrickvonplaten/subscriptions
User
https://api.github.com/users/patrickvonplaten
public
CONTRIBUTOR
I have an initial version here: https://github.com/EntilZha/nlp/tree/master/datasets/qanta Thats pretty close to what I'll do as a PR, but still want to do some more sanity checks/tests (just got tests passing).

I figured out how to get all tests passing by adding a download command and some finagling with the data zip https://github.com/EntilZha/nlp/blob/master/tests/utils.py#L127


2020-05-18T21:34:01Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630447242
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDQ0NzI0Mg==
https://api.github.com/repos/huggingface/datasets/issues/comments/630447242/reactions
2020-05-18T21:34:01Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630447242
https://avatars.githubusercontent.com/u/1382460?v=4
https://api.github.com/users/EntilZha/events{/privacy}
https://api.github.com/users/EntilZha/followers
https://api.github.com/users/EntilZha/following{/other_user}
https://api.github.com/users/EntilZha/gists{/gist_id}
https://github.com/EntilZha
EntilZha
MDQ6VXNlcjEzODI0NjA=
https://api.github.com/users/EntilZha/orgs
https://api.github.com/users/EntilZha/received_events
https://api.github.com/users/EntilZha/repos
https://api.github.com/users/EntilZha/starred{/owner}{/repo}
https://api.github.com/users/EntilZha/subscriptions
User
https://api.github.com/users/EntilZha
public
CONTRIBUTOR
I'm quite positive that you can just replace the `dl_manager.download()` statements here: https://github.com/EntilZha/nlp/blob/4d46443b65f1f756921db8275594e6af008a1de7/datasets/qanta/qanta.py#L194 with `dl_manager.download_and_extract()` even though you don't extract anything. I would prefer to avoid adding more functions to the MockDataLoadManager and keep it as simple as possible (It's already to complex now IMO). 

Could you check if you can replace the `download()` function? 
2020-05-18T21:56:24Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630455761
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDQ1NTc2MQ==
https://api.github.com/repos/huggingface/datasets/issues/comments/630455761/reactions
2020-05-18T21:56:24Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630455761
https://avatars.githubusercontent.com/u/23423619?v=4
https://api.github.com/users/patrickvonplaten/events{/privacy}
https://api.github.com/users/patrickvonplaten/followers
https://api.github.com/users/patrickvonplaten/following{/other_user}
https://api.github.com/users/patrickvonplaten/gists{/gist_id}
https://github.com/patrickvonplaten
patrickvonplaten
MDQ6VXNlcjIzNDIzNjE5
https://api.github.com/users/patrickvonplaten/orgs
https://api.github.com/users/patrickvonplaten/received_events
https://api.github.com/users/patrickvonplaten/repos
https://api.github.com/users/patrickvonplaten/starred{/owner}{/repo}
https://api.github.com/users/patrickvonplaten/subscriptions
User
https://api.github.com/users/patrickvonplaten
public
CONTRIBUTOR
I might be doing something wrong, but swapping those two gives this error:
```
>       with open(path) as f:
E       IsADirectoryError: [Errno 21] Is a directory: 'datasets/qanta/dummy/mode=first,char_skip=25/2018.4.18/dummy_data-zip-extracted/dummy_data'

src/nlp/datasets/qanta/3d965403133687b819905ead4b69af7bcee365865279b2f797c79f809b4490c3/qanta.py:280: IsADirectoryError

During handling of the above exception, another exception occurred:
```

So it seems like the directory name is getting passed. Is this not functioning as expected, or is there some caching happening maybe? I deleted the dummy files and re-ran the import script with no changes. I'm digging a bit in with a debugger, but no clear reason yet
2020-05-18T23:49:51Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630493264
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDQ5MzI2NA==
https://api.github.com/repos/huggingface/datasets/issues/comments/630493264/reactions
2020-05-18T23:49:51Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630493264
https://avatars.githubusercontent.com/u/1382460?v=4
https://api.github.com/users/EntilZha/events{/privacy}
https://api.github.com/users/EntilZha/followers
https://api.github.com/users/EntilZha/following{/other_user}
https://api.github.com/users/EntilZha/gists{/gist_id}
https://github.com/EntilZha
EntilZha
MDQ6VXNlcjEzODI0NjA=
https://api.github.com/users/EntilZha/orgs
https://api.github.com/users/EntilZha/received_events
https://api.github.com/users/EntilZha/repos
https://api.github.com/users/EntilZha/starred{/owner}{/repo}
https://api.github.com/users/EntilZha/subscriptions
User
https://api.github.com/users/EntilZha
public
CONTRIBUTOR
From what I can tell here: https://github.com/huggingface/nlp/blob/master/tests/utils.py#L115

1. `data_url` is the correct http link
2. `path_to_dummy_data` is a directory, which is causing the issue

That path comes from `download_dummy_data`, which I think assumes that the data comes from the zip file, but isn't aware of individual files. So it seems like it data manager needs to be aware if the url its getting is for a file or a zip/directory, and pass this information along. This might happen in `download_dummy_data`, but probably better to happen in `download_and_extract`? Maybe a simple check to see if `os.path.basename` returns the dummy data zip filename, if not then join paths with the basename of the url?
2020-05-18T23:56:58Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630495203
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDQ5NTIwMw==
https://api.github.com/repos/huggingface/datasets/issues/comments/630495203/reactions
2020-05-18T23:56:58Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630495203
https://avatars.githubusercontent.com/u/1382460?v=4
https://api.github.com/users/EntilZha/events{/privacy}
https://api.github.com/users/EntilZha/followers
https://api.github.com/users/EntilZha/following{/other_user}
https://api.github.com/users/EntilZha/gists{/gist_id}
https://github.com/EntilZha
EntilZha
MDQ6VXNlcjEzODI0NjA=
https://api.github.com/users/EntilZha/orgs
https://api.github.com/users/EntilZha/received_events
https://api.github.com/users/EntilZha/repos
https://api.github.com/users/EntilZha/starred{/owner}{/repo}
https://api.github.com/users/EntilZha/subscriptions
User
https://api.github.com/users/EntilZha
public
CONTRIBUTOR
I think the dataset script works correctly. Just the dummy data structure seems to be wrong. I will soon add more commands that should make the create of the dummy data easier.

I'd recommend that you won't concentrate too much on the dummy data.
If you manage to load the dataset correctly via:

```python 
# use local path to qanta
nlp.load_dataset("./datasets/qanta")
```

then feel free to open a PR and we will look into the dummy data problem together :-) 

Also please make sure that the Version is in the format 1.0.0 (three numbers separated by two points) - not a date. 
2020-05-19T09:04:40Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630689688
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDY4OTY4OA==
https://api.github.com/repos/huggingface/datasets/issues/comments/630689688/reactions
2020-05-19T09:05:41Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630689688
https://avatars.githubusercontent.com/u/23423619?v=4
https://api.github.com/users/patrickvonplaten/events{/privacy}
https://api.github.com/users/patrickvonplaten/followers
https://api.github.com/users/patrickvonplaten/following{/other_user}
https://api.github.com/users/patrickvonplaten/gists{/gist_id}
https://github.com/patrickvonplaten
patrickvonplaten
MDQ6VXNlcjIzNDIzNjE5
https://api.github.com/users/patrickvonplaten/orgs
https://api.github.com/users/patrickvonplaten/received_events
https://api.github.com/users/patrickvonplaten/repos
https://api.github.com/users/patrickvonplaten/starred{/owner}{/repo}
https://api.github.com/users/patrickvonplaten/subscriptions
User
https://api.github.com/users/patrickvonplaten
public
CONTRIBUTOR
The script loading seems to work fine so I'll work on getting a PR open after a few sanity checks on the data.

On version, we currently have it versioned with YYYY.MM.DD scheme so it would be nice to not change that, but will it cause issues?
2020-05-19T15:33:45Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630901229
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDkwMTIyOQ==
https://api.github.com/repos/huggingface/datasets/issues/comments/630901229/reactions
2020-05-19T15:33:45Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630901229
https://avatars.githubusercontent.com/u/1382460?v=4
https://api.github.com/users/EntilZha/events{/privacy}
https://api.github.com/users/EntilZha/followers
https://api.github.com/users/EntilZha/following{/other_user}
https://api.github.com/users/EntilZha/gists{/gist_id}
https://github.com/EntilZha
EntilZha
MDQ6VXNlcjEzODI0NjA=
https://api.github.com/users/EntilZha/orgs
https://api.github.com/users/EntilZha/received_events
https://api.github.com/users/EntilZha/repos
https://api.github.com/users/EntilZha/starred{/owner}{/repo}
https://api.github.com/users/EntilZha/subscriptions
User
https://api.github.com/users/EntilZha
public
CONTRIBUTOR
> The script loading seems to work fine so I'll work on getting a PR open after a few sanity checks on the data.
> 
> On version, we currently have it versioned with YYYY.MM.DD scheme so it would be nice to not change that, but will it cause issues?

It would cause issues for sure for the tests....not sure if it would also cause issues otherwise.

I would prefer to keep the same version style as we have for other models. You could for example simply add version 1.0.0 and add a comment with the date you currently use for the versioning.

 What is your opinion regarding the version here @lhoestq @mariamabarham @thomwolf ? 
2020-05-19T16:18:49Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630927183
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDkyNzE4Mw==
https://api.github.com/repos/huggingface/datasets/issues/comments/630927183/reactions
2020-05-19T16:19:44Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630927183
https://avatars.githubusercontent.com/u/23423619?v=4
https://api.github.com/users/patrickvonplaten/events{/privacy}
https://api.github.com/users/patrickvonplaten/followers
https://api.github.com/users/patrickvonplaten/following{/other_user}
https://api.github.com/users/patrickvonplaten/gists{/gist_id}
https://github.com/patrickvonplaten
patrickvonplaten
MDQ6VXNlcjIzNDIzNjE5
https://api.github.com/users/patrickvonplaten/orgs
https://api.github.com/users/patrickvonplaten/received_events
https://api.github.com/users/patrickvonplaten/repos
https://api.github.com/users/patrickvonplaten/starred{/owner}{/repo}
https://api.github.com/users/patrickvonplaten/subscriptions
User
https://api.github.com/users/patrickvonplaten
public
MEMBER
Maybe use the YYYY.MM.DD as the config name ? That's what we are doing for wikipedia
2020-05-19T16:39:32Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630940195
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDk0MDE5NQ==
https://api.github.com/repos/huggingface/datasets/issues/comments/630940195/reactions
2020-05-19T16:39:32Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630940195
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
CONTRIBUTOR
> Maybe use the YYYY.MM.DD as the config name ? That's what we are doing for wikipedia

I'm not sure if this will work because the name should be unique and it seems that he has multiple config name in his data with the same version.
As @patrickvonplaten  suggested, I think  you can add a comment about the version in the data description.
2020-05-19T17:00:28Z
https://github.com/huggingface/datasets/issues/161#issuecomment-630951479
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMDk1MTQ3OQ==
https://api.github.com/repos/huggingface/datasets/issues/comments/630951479/reactions
2020-05-19T17:00:28Z
https://api.github.com/repos/huggingface/datasets/issues/comments/630951479
https://avatars.githubusercontent.com/u/38249783?v=4
https://api.github.com/users/mariamabarham/events{/privacy}
https://api.github.com/users/mariamabarham/followers
https://api.github.com/users/mariamabarham/following{/other_user}
https://api.github.com/users/mariamabarham/gists{/gist_id}
https://github.com/mariamabarham
mariamabarham
MDQ6VXNlcjM4MjQ5Nzgz
https://api.github.com/users/mariamabarham/orgs
https://api.github.com/users/mariamabarham/received_events
https://api.github.com/users/mariamabarham/repos
https://api.github.com/users/mariamabarham/starred{/owner}{/repo}
https://api.github.com/users/mariamabarham/subscriptions
User
https://api.github.com/users/mariamabarham
public
MEMBER
Actually maybe our versioning format (inherited from tfds) is too strong for what we use it for?
We could allow any string maybe?

I see it more and more like an identifier for the user that we will back with a serious hashing/versioning system.- so we could let the user quite free on it.
2020-05-19T21:37:09Z
https://github.com/huggingface/datasets/issues/161#issuecomment-631096843
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMTA5Njg0Mw==
https://api.github.com/repos/huggingface/datasets/issues/comments/631096843/reactions
2020-05-19T21:38:17Z
https://api.github.com/repos/huggingface/datasets/issues/comments/631096843
https://avatars.githubusercontent.com/u/7353373?v=4
https://api.github.com/users/thomwolf/events{/privacy}
https://api.github.com/users/thomwolf/followers
https://api.github.com/users/thomwolf/following{/other_user}
https://api.github.com/users/thomwolf/gists{/gist_id}
https://github.com/thomwolf
thomwolf
MDQ6VXNlcjczNTMzNzM=
https://api.github.com/users/thomwolf/orgs
https://api.github.com/users/thomwolf/received_events
https://api.github.com/users/thomwolf/repos
https://api.github.com/users/thomwolf/starred{/owner}{/repo}
https://api.github.com/users/thomwolf/subscriptions
User
https://api.github.com/users/thomwolf
public
CONTRIBUTOR
I'm good with either putting it in description, adding it to the config, or loosening version formatting. I mostly don't have a full conceptual grasp of what each identifier ends up meaning in the datasets code so hard to evaluate the best approach.

For background, the multiple formats is a consequence of:

1. Each example is one multi-sentence trivia question
2. For training, its better to treat each sentence as an example
3. For evaluation, should test on: (1) first sentence, (2) full question, and (3) partial questions (does the model get the question right having seen the first half)

We use the date format for version since: (1) we expect some degree of updates since new questions come in every year and (2) the timestamp itself matches the Wikipedia dump that it is dependent on (so similar to the Wikipedia dataset).

perhaps this is better discussed in https://github.com/huggingface/nlp/pull/169 or update title?
2020-05-19T23:11:04Z
https://github.com/huggingface/datasets/issues/161#issuecomment-631134963
https://api.github.com/repos/huggingface/datasets/issues/161
MDEyOklzc3VlQ29tbWVudDYzMTEzNDk2Mw==
https://api.github.com/repos/huggingface/datasets/issues/comments/631134963/reactions
2020-05-19T23:11:04Z
https://api.github.com/repos/huggingface/datasets/issues/comments/631134963
https://avatars.githubusercontent.com/u/1382460?v=4
https://api.github.com/users/EntilZha/events{/privacy}
https://api.github.com/users/EntilZha/followers
https://api.github.com/users/EntilZha/following{/other_user}
https://api.github.com/users/EntilZha/gists{/gist_id}
https://github.com/EntilZha
EntilZha
MDQ6VXNlcjEzODI0NjA=
https://api.github.com/users/EntilZha/orgs
https://api.github.com/users/EntilZha/received_events
https://api.github.com/users/EntilZha/repos
https://api.github.com/users/EntilZha/starred{/owner}{/repo}
https://api.github.com/users/EntilZha/subscriptions
User
https://api.github.com/users/EntilZha
public
Hi, I'm working on adding a dataset and ran into an error due to `download` not being defined on `MockDataLoaderManager`, but being defined in `nlp/utils/download_manager.py`. The readme step running this: `RUN_SLOW=1 pytest tests/test_dataset_common.py::DatasetTest::test_load_real_dataset_localmydatasetname` triggers the error. If I can get something to work, I can include it in my data PR once I'm done.