t20062443/AT1ds huggingface dataset:
vegarab (CONTRIBUTOR) commented on 2020-07-20 10:32:15:
@lhoestq 
For some reason, I am not able to reproduce the segmentation fault, on pyarrow==0.16.0. Using the exact same environment and file.

Anyhow, I discovered that pyarrow>=0.17.0 is required to read in a JSON file where the pandas structs contain lists. Otherwise, pyarrow complains when attempting to cast the struct:
```py
import nlp
>>> s = nlp.load_dataset("json", data_files="datasets/train-v2.0.json", field="data", split="train")
Using custom data configuration default
>>> s[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/vegarab/.conda/envs/torch/lib/python3.7/site-packages/nlp/arrow_dataset.py", line 558, in __getitem__
    format_kwargs=self._format_kwargs,
  File "/home/vegarab/.conda/envs/torch/lib/python3.7/site-packages/nlp/arrow_dataset.py", line 498, in _getitem
    outputs = self._unnest(self._data.slice(key, 1).to_pandas().to_dict("list"))
  File "pyarrow/array.pxi", line 559, in pyarrow.lib._PandasConvertible.to_pandas
  File "pyarrow/table.pxi", line 1367, in pyarrow.lib.Table._to_pandas
  File "/home/vegarab/.conda/envs/torch/lib/python3.7/site-packages/pyarrow/pandas_compat.py", line 766, in table_to_blockmanager
    blocks = _table_to_blocks(options, table, categories, ext_columns_dtypes)
  File "/home/vegarab/.conda/envs/torch/lib/python3.7/site-packages/pyarrow/pandas_compat.py", line 1101, in _table_to_blocks
    list(extension_columns.keys()))
  File "pyarrow/table.pxi", line 881, in pyarrow.lib.table_to_blocks
  File "pyarrow/error.pxi", line 105, in pyarrow.lib.check_status
pyarrow.lib.ArrowNotImplementedError: Not implemented type for Arrow list to pandas: struct<qas: list<item: struct<question: string, id: string, answers: list<item: struct<text: string, answer_start: int64>>, is_impossible: bool, plausible_answers: list<item: struct<text: string, answer_start: int64>>>>, context: string>
>>> s
Dataset(schema: {'title': 'string', 'paragraphs': 'list<item: struct<qas: list<item: struct<question: string, id: string, answers: list<item: struct<text: string, answer_start: int64>>, is_impossible: bool, plausible_answers: list<item: struct<text: string, answer_start: int64>>>>, context: string>>'}, num_rows: 35)
```

Upgrading to >=0.17.0 provides the same dataset structure, but accessing the records is possible without the same exception.
lhoestq (MEMBER) commented on 2020-07-20 10:34:23:
I tried using `.take` from pyarrow recordbatches but it doesn't improve the speed that much:
```python
import nlp
import numpy as np

dset = nlp.Dataset.from_file("dummy_test_select.arrow")  # dummy dataset with 100000 examples like {"a": "h"*512}
indices = np.random.randint(0, 100_000, 1000_000)
```

```python
%%time
batch_size = 10_000
writer = ArrowWriter(schema=dset.schema, path="dummy_path",
                     writer_batch_size=1000, disable_nullable=False)
for i in tqdm(range(0, len(indices), batch_size)):
    table = pa.concat_tables(dset._data.slice(int(i), 1) for i in indices[i : min(len(indices), i + batch_size)])
    batch = table.to_pydict()
    writer.write_batch(batch)
writer.finalize()
# 9.12s
```


```python
%%time
batch_size = 10_000
writer = ArrowWriter(schema=dset.schema, path="dummy_path", 
                     writer_batch_size=1000, disable_nullable=False)
for i in tqdm(range(0, len(indices), batch_size)):
    batch_indices = indices[i : min(len(indices), i + batch_size)]
    # First, extract only the indices that we need with a mask
    mask = [False] * len(dset)
    for k in batch_indices:
        mask[k] = True
    t_batch = dset._data.filter(pa.array(mask))
    # Second, build the list of indices for the filtered table, and taking care of duplicates
    rev_positions = {}
    duplicates = 0
    for i, j in enumerate(sorted(batch_indices)):
        if j in rev_positions:
            duplicates += 1
        else:
            rev_positions[j] = i - duplicates
    rev_map = [rev_positions[j] for j in batch_indices]
    # Third, use `.take` from the combined recordbatch
    t_combined = t_batch.combine_chunks()  # load in memory
    recordbatch = t_combined.to_batches()[0]
    table = pa.Table.from_arrays(
        [recordbatch[c].take(pa.array(rev_map)) for c in range(len(dset._data.column_names))],
        schema=writer.schema
    )
    writer.write_table(table)
writer.finalize()
# 3.2s
```
lhoestq (MEMBER) commented on 2020-07-20 14:28:12:
I played a little bit with the code and it works quite well :)

I found two cases for which it doesn't work though:
- if the features dict depth is > 2 (ex: wikisql), because `flatten` only flattens the first level of nesting (it can be fixed by calling `flatten` several times in a row, see [here](https://issues.apache.org/jira/browse/ARROW-4090))
- Or if there are 2d features (ex: wikisql, `table.rows` is a sequence of sequences of strings), because tf.train.Features only support 1-d lists. That's why tensorflow-datasets flattens these 2-d features to 1-d and adds ragged features that are the shapes of the arrays, so that they can be reconstructed.

I think we can ignore the 2d stuff right now (some work is being done in #363 ), but I'd like to see the `flatten` issue fixed soon
hackerxiaobai (NONE) commented on 2020-07-21 02:21:10:
> `from_dict` was added in #350 that was unfortunately not included in the 0.3.0 release. It's going to be included in the next release that will be out pretty soon though.
> Right now if you want to use `from_dict` you have to install the package from the master branch
> 
> ```
> pip install git+https://github.com/huggingface/nlp.git
> ```
OK, thank you.
tholor (NONE) commented on 2020-07-21 06:36:37:
Ok, got it. I think this could be a valuable feature - especially for large datasets like NQ, but potentially also others. 
For us, it will in this case make the difference of using the library or keeping the old downloads of the raw dev datasets.  
However, I don't know if that fits into your plans with the library and can also understand if you don't want to support this.
lhoestq (MEMBER) commented on 2020-07-21 08:30:39:
I made `.flatten` work on our side (it calls pyarrow's flatten several times until it's really flat).

The only datasets that won't work are those with lists of lists of features, which is a rare case. Hopefully we can make this work with the multi-dimensional arrays changes we're also doing.

I think we can merge now :) cc @thomwolf
lhoestq (MEMBER) commented on 2020-07-21 08:44:57:
> I think when trying to cast an extension array within a list of dictionaries, some method gets called that bugs out Arrow and somehow doesn't get called when adding a single row to a a table and then appending multiple tables together. I tinkered with this for a while but could not find any workaround.

Indeed that's weird.

> In the case that this new method causes bad compression/worse performance, we can explicitly set the batch size in the pa.Table.to_batches(batch_size) method, which will return a list of batches. Perhaps, we can check that the batch size is not too large converting the table to batches after X many rows are appended to it by following the batch_size check below.

The argument of `pa.Table.to_batches` is not `batch_size` but `max_chunksize`, which means that right now it would have no effects (each chunk is of length 1).

We can fix that just by doing `entries.combine_chunks().to_batches(batch_size)`. In that case it would write by chunk of 1000 which is what we want. I don't think it will slow down the writing by much, but we may have to do a benchmark just to make sure. If speed is ok we could even replace the original code to always write chunks this way.

Do you still have errors that need to be fixed ?
ghazi-f (CONTRIBUTOR) commented on 2020-07-21 13:10:42:
Hi there !
I encountered the same issue with the IMDB dataset on windows. It threw an error about charmap not being able to decode a symbol during the first time I tried to download it. I checked on a remote linux machine I have, and it can't be reproduced.
I added ```encoding='UTF-8'``` to both lines that have ```open``` in ```imdb.py```  (108 and 114) and it worked for me.
Thank you !
lhoestq (MEMBER) commented on 2020-07-21 13:12:51:
> Hi there !
> I encountered the same issue with the IMDB dataset on windows. It threw an error about charmap not being able to decode a symbol during the first time I tried to download it. I checked on a remote linux machine I have, and it can't be reproduced.
> I added `encoding='UTF-8'` to both lines that have `open` in `imdb.py` (108 and 114) and it worked for me.
> Thank you !

Hello !
Glad you managed to fix this issue on your side.
Do you mind opening a PR for IMDB ?
jarednielsen (CONTRIBUTOR) commented on 2020-07-23 00:20:31:
This PR could be useful. My specific use case is `multiprocessing.Pool` for parallel preprocessing (because of the Python tokenization bottleneck at https://github.com/huggingface/transformers/issues/5729). I shard a large dataset, run map on each shard within a multiprocessing pool, and then concatenate them back together. This is only possible if a dataset can be pickled, otherwise the logic is much more complex. There's no reason to make it un-picklable, even if it's not the recommended usage.

```python
import nlp
import multiprocessing

def func(ex):
    return {"text": "Prefix: " + ex["text"]}

def map_helper(dset):
    return dset.map(func)

n_shards = 16
dset = nlp.load_dataset("wikitext-2-raw-v1", split="train")
with multiprocessing.Pool(processes=n_shards) as pool:
    shards = pool.map(map_helper, [dset.shard(n_shards, i, contiguous=True) for i in range(n_shards)])
dset = nlp.concatenate_datasets(shards)
```
lvwerra (MEMBER) commented on 2020-07-23 10:42:57:
Hi @lhoestq 
I made the proposed changes to the `xtreme.py` script. I noticed that I also need to change the schema in the `dataset_infos.json` file.  More specifically the `"features"` part of the PAN-X.LANG dataset:

```json
"features":{
   "word":{
      "dtype":"string",
      "id":null,
      "_type":"Value"
   },
   "ner_tag":{
      "dtype":"string",
      "id":null,
      "_type":"Value"
   },
   "lang":{
      "dtype":"string",
      "id":null,
      "_type":"Value"
   }
}
```
To fit the code above the fields `"word"`, `"ner_tag"`, and `"lang"` would become `"words"`, `ner_tags"` and `"langs"`. In addition the `dtype` should be changed from `"string"` to `"list"`.

 I made this changes but when trying to test this locally with `dataset = load_dataset("xtreme", "PAN-X.en", data_dir='./data')` I face the issue that the `dataset_info.json` file is always overwritten by a downloaded version with the old settings, which then throws an error because the schema does not match. This makes it hard to test the changes locally. Do you have any suggestions on how to deal with that?
mariamabarham (CONTRIBUTOR) commented on 2020-07-23 12:08:34:
Thanks @RachelKer for this PR.

I think the dummy_data structure does not also match. In the `_split_generator` you have something like `os.path.join(downloaded_files["validation"], lang+'_val.jsonl')` but in you dummy_data you have `os.path.join(downloaded_files["validation"], lang+"_val.zip",  lang+'_val.jsonl')`. I think ` jsonl` files should be directly  in the `dummy_data` folder without the sub-folder 

@lhoestq
lhoestq (MEMBER) commented on 2020-07-23 16:18:40:
Hi !

You have to point to your local script.
First clone the repo and then:

```python
dataset = load_dataset("./datasets/xtreme", "PAN-X.en")
```
The "xtreme" directory contains "xtreme.py".

You also have to change the features definition in the `_info` method. You could use:

```python
features = nlp.Features({
    "words": [nlp.Value("string")],
    "ner_tags": [nlp.Value("string")],
    "langs": [nlp.Value("string")],
})
```

Hope this helps !
Let me know if you have other questions.
lhoestq (MEMBER) commented on 2020-07-23 16:40:33:
> Is there a way to convert a dataset to Apache arrow format (locally/personal use) & use it before sending it to hugging face?

Sure, to get a dataset in arrow format you can either:
- [load from local files (txt, json, csv)](https://huggingface.co/nlp/loading_datasets.html?highlight=csv#from-local-files)
- OR [load from python data (dict, pandas)](https://huggingface.co/nlp/loading_datasets.html?highlight=csv#from-in-memory-data)
- OR [create your own dataset script](https://huggingface.co/nlp/loading_datasets.html?highlight=csv#using-a-custom-dataset-loading-script)
lvwerra (MEMBER) commented on 2020-07-23 16:50:58:
Thanks, I am making progress. I got a new error `NonMatchingSplitsSizesError ` (see traceback below), which I suspect is due to the fact that number of rows in the dataset changed (one row per word --> one row per sentence) as well as the number of bytes due to the slightly updated data structure. 

```python
NonMatchingSplitsSizesError: [{'expected': SplitInfo(name='validation', num_bytes=1756492, num_examples=80536, dataset_name='xtreme'), 'recorded': SplitInfo(name='validation', num_bytes=1837109, num_examples=10000, dataset_name='xtreme')}, {'expected': SplitInfo(name='test', num_bytes=1752572, num_examples=80326, dataset_name='xtreme'), 'recorded': SplitInfo(name='test', num_bytes=1833214, num_examples=10000, dataset_name='xtreme')}, {'expected': SplitInfo(name='train', num_bytes=3496832, num_examples=160394, dataset_name='xtreme'), 'recorded': SplitInfo(name='train', num_bytes=3658428, num_examples=20000, dataset_name='xtreme')}]
```
I can fix the error by replacing the values in the `datasets_infos.json` file, which I tested for English. However, to update this for all 40 datasets manually is slightly painful. Is there a better way to update the expected values for all datasets?
lhoestq (MEMBER) commented on 2020-07-24 12:16:56:
One more thing about features. I mentioned

```python
features = nlp.Features({
    "words": [nlp.Value("string")],
    "ner_tags": [nlp.Value("string")],
    "langs": [nlp.Value("string")],
})
```

but it's actually not consistent with the way we write datasets. Something like this is simpler to read and more consistent with the way we define datasets:

```python
features = nlp.Features({
    "words": nlp.Sequence(nlp.Value("string")),
    "ner_tags": nlp.Sequence(nlp.Value("string")),
    "langs": nlp.Sequence(nlp.Value("string")),
})
```

Sorry about that
lhoestq (MEMBER) commented on 2020-07-24 12:24:39:
> @lhoestq Nope all should be good!

Awesome :)

I think it would be good to start to add some tests then.
You already have `test_multi_array.py` which is a good start, maybe you can place it in /tests and make it a `unittest.TestCase` ?

> Would you like me to add the entries.combine_chunks().to_batch_size() code + benchmark?

That would be interesting. We don't want reading/writing to be the bottleneck of dataset processing for example in terms of speed. Maybe we could test the write + read speed of different datasets:
- write speed + read speed a dataset with `nlp.Array2D` features
- write speed + read speed a dataset with `nlp.Sequence(nlp.Sequence(nlp.Value("float32")))` features
- write speed + read speed a dataset with `nlp.Sequence(nlp.Value("float32"))` features (same data but flatten)
It will be interesting to see the influence of `.combine_chunks()` on the `Array2D` test too.

What do you think ?
lhoestq (MEMBER) commented on 2020-07-24 14:57:13:
I just tested your code to try to understand better.


- First thing you must know is that we've switched from `dataset._data.to_pandas` to `dataset._data.to_pydict` by default when we call `dataset[0]` in #423 . Right now it raises an error but it can be fixed by adding this method to `ExtensionArray2D`:

```python
    def to_pylist(self):
        return self.to_numpy().tolist()
```

- Second, I noticed that `ExtensionArray2D.to_numpy()` always return a (5, 5) shape in your example. I thought `ExtensionArray` was for possibly multiple examples and so I was expecting a shape like (1, 5, 5) for example. Did I miss something ?
Therefore when I apply the fix I mentioned (adding to_pylist), it returns one example per row in each image (in your example of 2 images of shape 5x5, I get `len(dataset._data.to_pydict()["image"])  == 10  # True`)

[EDIT] I changed the reshape step in `ExtensionArray2D.to_numpy()` by
```python
numpy_arr = numpy_arr.reshape(len(self), *ExtensionArray2D._construct_shape(self.storage))
```
and it did the job: `len(dataset._data.to_pydict()["image"])  == 2  # True`

- Finally, I was able to make `to_pandas` work though, by implementing custom array dtype in pandas with arrow conversion (I got inspiration from [here](https://gist.github.com/Eastsun/a59fb0438f65e8643cd61d8c98ec4c08) and [here](https://pandas.pydata.org/pandas-docs/version/1.0.0/development/extending.html#compatibility-with-apache-arrow))

Maybe you could add me in your repo so I can open a PR to add these changes to your branch ?
eltoto1219 (CONTRIBUTOR) commented on 2020-07-24 20:09:09:
> > @lhoestq Nope all should be good!
> 
> Awesome :)
> 
> I think it would be good to start to add some tests then.
> You already have `test_multi_array.py` which is a good start, maybe you can place it in /tests and make it a `unittest.TestCase` ?
> 
> > Would you like me to add the entries.combine_chunks().to_batch_size() code + benchmark?
> 
> That would be interesting. We don't want reading/writing to be the bottleneck of dataset processing for example in terms of speed. Maybe we could test the write + read speed of different datasets:
> 
> * write speed + read speed a dataset with `nlp.Array2D` features
> * write speed + read speed a dataset with `nlp.Sequence(nlp.Sequence(nlp.Value("float32")))` features
> * write speed + read speed a dataset with `nlp.Sequence(nlp.Value("float32"))` features (same data but flatten)
>   It will be interesting to see the influence of `.combine_chunks()` on the `Array2D` test too.
> 
> What do you think ?

Ya! that should be no problem at all, Ill use the timeit module and get back to you with the results sometime over the weekend.
eltoto1219 (CONTRIBUTOR) commented on 2020-07-24 20:16:34:
Thank you for all your help getting the pandas and row indexing for the dataset to work! For `print(dataset[0])`, I considered the workaround of doing  `print(dataset["col_name"][0])` a temporary solution, but ya, I was never able to figure out how to previously get it to work. I'll add you to my repo right now, let me know if you do not see the invite. Also lastly, it is strange how the to_batches method is not working, so I can check that out while I add some speed tests + add the multi dim test under the unit tests this weekend.
timothyjlaurent (NONE) commented on 2020-07-25 03:15:45:
So `tf.data.Dataset.map()` returns a `ParallelMapDataset` if `num_parallel_calls is not None` [link](https://github.com/tensorflow/tensorflow/blob/2b96f3662bd776e277f86997659e61046b56c315/tensorflow/python/data/ops/dataset_ops.py#L1623).

There, `num_parallel_calls` is turned into a tensor and and fed to `gen_dataset_ops.parallel_map_dataset` where it looks like tensorflow takes over.

We could start with something simple like a thread or process pool that `imap`s over some shards.
jxmorris12 (CONTRIBUTOR) commented on 2020-07-25 15:38:30:
+1! this is the reason our tests are failing at [TextAttack](https://github.com/QData/TextAttack) 

(Though it's worth noting if we fixed the version number of pyarrow to 0.16.0 that would fix our problem too. But in this case we'll just wait for you all to update)
svenski (NONE) commented on 2020-07-25 16:03:45:
Came to raise this issue, great to see other already have and it's being fixed so soon!

As an aside, since no one wrote this already, it seems like the version check only looks at the second part of the version number making sure it is >16, but pyarrow newest version is 1.0.0 so the second past is 0!
thomwolf (MEMBER) commented on 2020-07-26 12:39:01:
There is an interesting design question here (cc @lhoestq).

I guess the labels form a closed set so we could also use a [nlp.ClassLabel](https://huggingface.co/nlp/package_reference/main_classes.html#nlp.ClassLabel) instead of a string. The differences will be mainly that:
- the labels are stored as integers and thus ready for training a model
- the string to int conversion methods are handled by the `nlp.ClassLabel` feature (see the [doc](https://huggingface.co/nlp/package_reference/main_classes.html#nlp.ClassLabel)  and [here](https://huggingface.co/nlp/features.html)  and [here](https://huggingface.co/nlp/quicktour.html#fine-tuning-a-deep-learning-model)).

In my opinion, storing the labels as integers instead of strings makes it:
- slightly less readable when accessing a dataset example (e.g. with `dataset[0]`)
- force you with a specific mapping from string to integers
- more clear that there is a fixed and predefined list of labels
- easier to list all the labels (directly visible in the features).

=> overall I'm pretty neutral about using one or the other option (`nlp.string` or `nlp.ClassLabel`).

Note that we can now rather easily convert from one to the other with the map function and something like:
```python
dataset = dataset.map(lambda x: x, features=nlp.Features({'labels': nlp.ClassLabel(MY_LABELS_NAMES)}))
dataset = dataset.map(lambda x: {'labels': dataset.features['labels'].int2str(x['labels'])}, features=nlp.Features({'labels': nlp.Value('string')}))
```
^^ this could probably be made even simpler (in particular for the second case)
thomwolf (MEMBER) commented on 2020-07-26 22:22:19:
Thanks Sam, we now have a very detailed tutorial and template on how to add a new dataset to the library. It typically take 1-2 hours to add one. Do you want to give it a try ?
The tutorial on writing a new dataset loading script is here: https://huggingface.co/nlp/add_dataset.html
And the part on how to share a new dataset is here: https://huggingface.co/nlp/share_dataset.html
lhoestq (MEMBER) commented on 2020-07-27 09:12:58:
`DPRContextEncoder` and `DPRContextEncoderTokenizer` will be available in the next release of `transformers`.

Right now you can experiment with it by installing `transformers` from the master branch.
You can also check the docs of DPR [here](https://huggingface.co/transformers/master/model_doc/dpr.html).

Moreover all the indexing features will also be available in the next release of `nlp`.
lhoestq (MEMBER) commented on 2020-07-28 09:21:48:
I was using a hack in `wiki_dpr` to download the index from GCS even for the configurations without the embeddings.
However as GCS is something internal, I changed the logic to add a download step for indexes directly in the dataset script, using the `DownloadManager`.

This change was directly linked to the changes I did to take into account the split name in the post processing, so I included this change in this PR too.

To summarize:

Dataset builders can now implement
- `_post_processing_resources(split)`: return a dict `resource_name -> resource_file_name`. It defines the additional resources such as indexes or arrow files that you need in post processing
- `_download_post_processing_resources(split, resource_name, dl_manager))`: if some resources can be downloaded, you can use the download_manager to download them
- `_post_process(dataset, resources_path)`: (main function for post processing) given a dataset, you can apply dataset transforms or add indexes. For resources that have been downloaded, you can load them. For the others, you can generate and save them. The paths to load/save resources are in `resources_path` which is a dictionary `resource_name -> resource_path`

About the CI:
```
=========================== short test summary info ============================
FAILED tests/test_dataset_common.py::AWSDatasetTest::test_load_dataset_wiki_dpr
```
It fails because I changed the input of post processing functions (to include the split name)