konrud/llm_course_chapter5_own_dataset_github_issues huggingface dataset:
[Checksums] If no configs exist prevent to run over empty list

`movie_rationales` e.g. has no configs.

Whoops I fixed it directly on master before checking that you have done it in this PR. We may close it

Yeah, I saw :-) But I think we should add this as well since some datasets have an empty list of configs and then as the code is now it would fail. 

In this PR, I just make sure that the code jumps in the correct else if "there are no configs" as is the case for some datasets @mariamabarham 

Sorry, I thought you meant a different commit . Just saw this one: https://github.com/huggingface/nlp/commit/7c644f284e2303b57612a6e7c904fe13906d893f
.

All good then :-)
[Checksums] Error for some datasets

The checksums command works very nicely for `squad`. But for `crime_and_punish` and `xnli`, 
the same bug happens:

When running: 
```
python nlp-cli nlp-cli test xnli --save_checksums
```

leads to:

```
  File "nlp-cli", line 33, in <module>
    service.run()
  File "/home/patrick/python_bin/nlp/commands/test.py", line 61, in run
    ignore_checksums=self._ignore_checksums,
  File "/home/patrick/python_bin/nlp/builder.py", line 383, in download_and_prepare
    self._download_and_prepare(dl_manager=dl_manager, download_config=download_config)
  File "/home/patrick/python_bin/nlp/builder.py", line 627, in _download_and_prepare
    dl_manager=dl_manager, max_examples_per_split=download_config.max_examples_per_split,
  File "/home/patrick/python_bin/nlp/builder.py", line 431, in _download_and_prepare
    split_generators = self._split_generators(dl_manager, **split_generators_kwargs)
  File "/home/patrick/python_bin/nlp/datasets/xnli/8bf4185a2da1ef2a523186dd660d9adcf0946189e7fa5942ea31c63c07b68a7f/xnli.py", line 95, in _split_generators
    dl_dir = dl_manager.download_and_extract(_DATA_URL)
  File "/home/patrick/python_bin/nlp/utils/download_manager.py", line 246, in download_and_extract
    return self.extract(self.download(url_or_urls))
  File "/home/patrick/python_bin/nlp/utils/download_manager.py", line 186, in download
    self._record_sizes_checksums(url_or_urls, downloaded_path_or_paths)
  File "/home/patrick/python_bin/nlp/utils/download_manager.py", line 166, in _record_sizes_checksums
    self._recorded_sizes_checksums[url] = get_size_checksum(path)
  File "/home/patrick/python_bin/nlp/utils/checksums_utils.py", line 81, in get_size_checksum
    with open(path, "rb") as f:
TypeError: expected str, bytes or os.PathLike object, not tuple
```


@lhoestq - could you take a look? It's not very urgent though!

Fixed with 06882b4

Now your command works :)
Note that you can also do
```
nlp-cli test datasets/nlp/xnli --save_checksums
```
So that it will save the checksums directly in the right directory.

Awesome!
Cleanup Features - Updating convert command - Fix Download manager

This PR makes a number of changes:

# Updating `Features`

Features are a complex mechanism provided in `tfds` to be able to modify a dataset on-the-fly when serializing to disk and when loading from disk.

We don't really need this because (1) it hides too much from the user and (2) our datatype can be directly mapped to Arrow tables on drive so we usually don't need to change the format before/after serialization.

This PR extracts and refactors these features in a single `features.py` files. It still keep a number of features classes for easy compatibility with tfds, namely the `Sequence`, `Tensor`, `ClassLabel` and `Translation` features.

Some more complex features involving a pre-processing on-the-fly during serialization are kept:
- `ClassLabel` which are able to convert from label strings to integers,
- `Translation`which does some check on the languages.

# Updating the `convert` command

We do a few updates here
- following the simplification of the `features` (cf above), conversion are updated
- we also makes it simpler to convert a single file
- some code need to be fixed manually after conversion (e.g. to remove some encoding processing in former tfds `Text` features. We highlight this code with a "git merge conflict" style syntax for easy manual fixing.

# Fix download manager iterator

You kept me up quite late on Tuesday night with this `os.scandir` change @lhoestq ;-)


For conflicts, I think the mention hint "This should be modified because it mentions ..." is missing.

Looks great!
Replace tf.constant for TF

Replace simple tf.constant type of Tensor to tf.ragged.constant which allows to have examples of different size in a tf.data.Dataset.

Now the training works with TF. Here the same example than for the PT in collab:

```python
import tensorflow as tf
import nlp
from transformers import BertTokenizerFast, TFBertForQuestionAnswering

# Load our training dataset and tokenizer
train_dataset = nlp.load('squad', split="train[:1%]")
tokenizer = BertTokenizerFast.from_pretrained('bert-base-cased')

def get_correct_alignement(context, answer):
    start_idx = answer['answer_start'][0]
    text = answer['text'][0]
    end_idx = start_idx + len(text)
    if context[start_idx:end_idx] == text:
        return start_idx, end_idx       # When the gold label position is good
    elif context[start_idx-1:end_idx-1] == text:
        return start_idx-1, end_idx-1   # When the gold label is off by one character
    elif context[start_idx-2:end_idx-2] == text:
        return start_idx-2, end_idx-2   # When the gold label is off by two character
    else:
        raise ValueError()

# Tokenize our training dataset
def convert_to_features(example_batch):
    # Tokenize contexts and questions (as pairs of inputs)
    input_pairs = list(zip(example_batch['context'], example_batch['question']))
    encodings = tokenizer.batch_encode_plus(input_pairs, pad_to_max_length=True)

    # Compute start and end tokens for labels using Transformers's fast tokenizers alignement methods.
    start_positions, end_positions = [], []
    for i, (context, answer) in enumerate(zip(example_batch['context'], example_batch['answers'])):
        start_idx, end_idx = get_correct_alignement(context, answer)
        start_positions.append([encodings.char_to_token(i, start_idx)])
        end_positions.append([encodings.char_to_token(i, end_idx-1)])
    
    if start_positions and end_positions:
      encodings.update({'start_positions': start_positions,
                        'end_positions': end_positions})
    return encodings

train_dataset = train_dataset.map(convert_to_features, batched=True)

columns = ['input_ids', 'token_type_ids', 'attention_mask', 'start_positions', 'end_positions']
train_dataset.set_format(type='tensorflow', columns=columns)
features = {x: train_dataset[x] for x in columns[:3]} 
labels = {"output_1": train_dataset["start_positions"]}
labels["output_2"] = train_dataset["end_positions"]
tfdataset = tf.data.Dataset.from_tensor_slices((features, labels)).batch(8)
model = TFBertForQuestionAnswering.from_pretrained("bert-base-cased")
loss_fn = tf.keras.losses.SparseCategoricalCrossentropy(reduction=tf.keras.losses.Reduction.NONE, from_logits=True)
opt = tf.keras.optimizers.Adam(learning_rate=3e-5)
model.compile(optimizer=opt,
              loss={'output_1': loss_fn, 'output_2': loss_fn},
              loss_weights={'output_1': 1., 'output_2': 1.},
              metrics=['accuracy'])
model.fit(tfdataset, epochs=1, steps_per_epoch=3)
```

Awesome!
Updating caching mechanism - Allow dependency in dataset processing scripts - Fix style and quality in the repo

This PR has a lot of content (might be hard to review, sorry, in particular because I fixed the style in the repo at the same time).

# Style & quality:
You can now install the style and quality tools with `pip install -e .[quality]`. This will install black, the compatible version of sort and flake8.
You can then clean the style and check the quality before merging your PR with:
```bash
make style
make quality
```

# Allow dependencies in dataset processing scripts
We can now allow (some level) of imports in dataset processing scripts (in addition to PyPi imports).
Namely, you can do the two following things:

Import from a relative path to a file in the same folder as the dataset processing script:
```python
import .c4_utils 
``` 

Or import from a relative path to a file in a folder/archive/github repo to which you provide an URL after the import state with `# From: [URL]`:
```python
import .clicr.dataset_code.build_json_dataset  # From: https://github.com/clips/clicr
```

In both these cases, after downloading the main dataset processing script, we will identify the location of these dependencies, download them and copy them in the dataset processing script folder.

Note that only direct import in the dataset processing script will be handled.
We don't recursively explore the additional import to download further files.
Also, when we download from an additional directory (in the second case above), we recursively add `__init__.py` to all the sub-folder so you can import from them.

This part is still tested for now. If you've seen datasets which required external utilities, tell me and I can test it.

# Update the cache to have a better local structure

The local structure in the `src/datasets` folder is now: `src/datasets/DATASET_NAME/DATASET_HASH/*`

The hash is computed from the full code of the dataset processing script as well as all the local and downloaded dependencies as mentioned above. This way if you change some code in a utility related to your dataset, a new hash should be computed.

LGTM
create our own DownloadManager

I tried to create our own - and way simpler - download manager, by replacing all the complicated stuff with our own `cached_path` solution.
With this implementation, I tried `dataset = nlp.load('squad')` and it seems to work fine.

For the implementation, what I did exactly:
- I copied the old download manager
- I removed all the dependences to the old `download` files
- I replaced all the download + extract calls by calls to `cached_path`
- I removed unused parameters (extract_dir, compute_stats) (maybe compute_stats could be re-added later if we want to compute stats...)
- I left some functions unimplemented for now. We will probably have to implement them because they are used by some datasets scripts (download_kaggle_data, iter_archive) or because we may need them at some point (download_checksums, _record_sizes_checksums)

Let me know if you think that this is going the right direction or if you have remarks.
Note: I didn't write any test yet as I wanted to read your remarks first

Looks great to me! 

The new download manager is ready. I removed the old folder and I fixed a few remaining dependencies.
I tested it on squad and a few others from the dataset folder and it works fine.

The only impact of these changes is that it breaks the `download_and_prepare` script that was used to register the checksums when we create a dataset, as the checksum logic is not implemented.

Let me know if you have remarks

Ok merged it (a bit fast for you to update the copyright, now I see that. but it's ok, we'll do a pass on these doc/copyright before releasing anyway)

Actually two additional things here @lhoestq (I merged too fast sorry, let's make a new PR for additional developments):
- I think we can remove some dependencies now (e.g. `promises`) in setup.py, can you have a look?
- also, I think we can remove the boto3 dependency like here: https://github.com/huggingface/transformers/pull/3968
[Make style]

Added Makefile and applied make style to all. 
make style runs the following code:

```
style:
          black --line-length 119 --target-version py35 src
          isort --recursive src
```

It's the same code that is run in `transformers`.

I think this can be quickly reproduced. 
I use `black, version 19.10b0`. 

When running: 
`black nlp/src/arrow_reader.py` 
it gives me: 

```
error: cannot format /home/patrick/hugging_face/nlp/src/nlp/arrow_reader.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 78)
Oh no! 💥 💔 💥
1 file failed to reformat.
```

The line in question is: 
https://github.com/huggingface/nlp/blob/6922a16705e61f9e31a365f2606090b84d49241f/src/nlp/arrow_reader.py#L78

What is weird is that the trainer file in `transformers` has more or less the same syntax and black does not fail there: 
https://github.com/huggingface/transformers/blob/cb3c2212c79d7ff0a4a4e84c3db48371ecc1c15d/src/transformers/trainer.py#L95

I googled quite a bit about black & typing hints yesterday and didn't find anything useful. 
Any ideas @thomwolf @julien-c @LysandreJik ?

> I think this can be quickly reproduced.
> I use `black, version 19.10b0`.
> 
> When running:
> `black nlp/src/arrow_reader.py`
> it gives me:
> 
> ```
> error: cannot format /home/patrick/hugging_face/nlp/src/nlp/arrow_reader.py: cannot use --safe with this file; failed to parse source file.  AST error message: invalid syntax (<unknown>, line 78)
> Oh no! 💥 💔 💥
> 1 file failed to reformat.
> ```
> 
> The line in question is:
> https://github.com/huggingface/nlp/blob/6922a16705e61f9e31a365f2606090b84d49241f/src/nlp/arrow_reader.py#L78
> 
> What is weird is that the trainer file in `transformers` has more or less the same syntax and black does not fail there:
> https://github.com/huggingface/transformers/blob/cb3c2212c79d7ff0a4a4e84c3db48371ecc1c15d/src/transformers/trainer.py#L95
> 
> I googled quite a bit about black & typing hints yesterday and didn't find anything useful.
> Any ideas @thomwolf @julien-c @LysandreJik ?

Ok I found the problem. It was the one Julien mentioned and has nothing to do with this line. Black's error message is a bit misleading here, I guess

Ok, just had to remove the python 2 syntax comments `# type`. 

Good to merge for me now @thomwolf
[Map Function] add assert statement if map function does not return dict or None

IMO, if a function is provided that is not a print statement (-> returns variable of type `None`) or a function that updates the datasets (-> returns variable of type `dict`), then a `TypeError` should be raised. 

Not sure whether you had cases in mind where the user should do something else @thomwolf , but I think a lot of silent errors can be avoided with this assert statement.

Also added to an assert statement that if a dict is returned by function, all values of `dicts` are `lists`

Wait to merge until `make style` is set in place.

Updated the assert statements. Played around with multiple cases and it should be good now IMO.
[Clean up] Datasets

Clean up `nlp/datasets` folder. 

As I understood, eventually the `nlp/datasets` shall not exist anymore at all. 

The folder `nlp/datasets/nlp` is kept for the moment, but won't be needed in the future, since it will live on S3 (actually it already does) at: `https://s3.console.aws.amazon.com/s3/buckets/datasets.huggingface.co/nlp/?region=us-east-1` and the different `dataset downloader scripts will be added to `nlp/src/nlp` when downloaded by the user. 

The folder `nlp/datasets/checksums` is kept for now, but won't be needed anymore in the future. 

The remaining folders/ files are leftovers from tensorflow-datasets and are not needed. The can be looked up in the private tensorflow-dataset repo.

Yes!
Error when citation is not given in the DatasetInfo

The following error is raised when the `citation` parameter is missing when we instantiate a `DatasetInfo`:
```
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jplu/dev/jplu/datasets/src/nlp/info.py", line 338, in __repr__
    citation_pprint = _indent('"""{}"""'.format(self.citation.strip()))
AttributeError: 'NoneType' object has no attribute 'strip'
```

I propose to do the following change in the `info.py` file. The method:
```python
def __repr__(self):
        splits_pprint = _indent("\n".join(["{"] + [
                "    '{}': {},".format(k, split.num_examples)
                for k, split in sorted(self.splits.items())
        ] + ["}"]))
        features_pprint = _indent(repr(self.features))
        citation_pprint = _indent('"""{}"""'.format(self.citation.strip()))
        return INFO_STR.format(
                name=self.name,
                version=self.version,
                description=self.description,
                total_num_examples=self.splits.total_num_examples,
                features=features_pprint,
                splits=splits_pprint,
                citation=citation_pprint,
                homepage=self.homepage,
                supervised_keys=self.supervised_keys,
                # Proto add a \n that we strip.
                license=str(self.license).strip())
```
Becomes:
```python
def __repr__(self):
        splits_pprint = _indent("\n".join(["{"] + [
                "    '{}': {},".format(k, split.num_examples)
                for k, split in sorted(self.splits.items())
        ] + ["}"]))
        features_pprint = _indent(repr(self.features))
        ## the strip is done only is the citation is given
        citation_pprint = self.citation

        if self.citation:
            citation_pprint = _indent('"""{}"""'.format(self.citation.strip()))
        return INFO_STR.format(
                name=self.name,
                version=self.version,
                description=self.description,
                total_num_examples=self.splits.total_num_examples,
                features=features_pprint,
                splits=splits_pprint,
                citation=citation_pprint,
                homepage=self.homepage,
                supervised_keys=self.supervised_keys,
                # Proto add a \n that we strip.
                license=str(self.license).strip())
```
And now it is ok. @thomwolf are you ok with this fix?

Yes looks good to me.
Note that we may refactor quite strongly the `info.py` to make it a lot simpler (it's very complicated for basically a dictionary of info I think)

No, problem ^^ It might just be a temporary fix :)

Fixed.
[Feature] Keep the list of labels of a dataset as metadata

It would be useful to keep the list of the labels of a dataset as metadata. Either directly in the `DatasetInfo` or in the Arrow metadata.

Yes! I see mostly two options for this:
- a `Feature` approach like currently (but we might deprecate features)
- wrapping in a smart way the Dictionary arrays of Arrow: https://arrow.apache.org/docs/python/data.html?highlight=dictionary%20encode#dictionary-arrays

I would have a preference for the second bullet point.

This should be accessible now as a feature in dataset.info.features (and even have the mapping methods).

Perfect! Well done!!

Hi,
I hope we could get a better documentation.
It took me more than 1 hour to found this way to get the label information.

Yes we are working on the doc right now, should be in the next release quite soon.
[Feature] More dataset outputs

Add the following dataset outputs:

- Spark
- Pandas

Yes!
- pandas will be a one-liner in `arrow_dataset`: https://arrow.apache.org/docs/python/generated/pyarrow.Table.html#pyarrow.Table.to_pandas
- for Spark I have no idea. let's investigate that at some point

For Spark it looks to be pretty straightforward as well https://spark.apache.org/docs/latest/sql-pyspark-pandas-with-arrow.html but looks to be having a dependency to Spark is necessary, then nevermind we can skip it

Now Pandas is available.