:bug: #2096 in tokenizer.decode, adds a space after special tokens for string format
Author: mandubian
Pull request: yes

This correction is cosmetic to correct the observed formatting issue.
No test was implemented because ideally composition of functions `encode.decode` should in theory return the original sentence. Yet there are some space strip (and lower-casing) in code so it's not certain to return exactly the original sentence with same spaces.

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/2101?src=pr&el=h1) Report
> Merging [#2101](https://codecov.io/gh/huggingface/transformers/pull/2101?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/0cb163865a4c761c226b151283309eedb2b1ca4d?src=pr&el=desc) will **decrease** coverage by `2.58%`.
> The diff coverage is `19.23%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/transformers/pull/2101/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/transformers/pull/2101?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #2101      +/-   ##
==========================================
- Coverage   82.67%   80.08%   -2.59%     
==========================================
  Files         111      112       +1     
  Lines       16162    16874     +712     
==========================================
+ Hits        13362    13514     +152     
- Misses       2800     3360     +560
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/2101?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [transformers/tests/tokenization\_bert\_test.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL3Rlc3RzL3Rva2VuaXphdGlvbl9iZXJ0X3Rlc3QucHk=) | `89.47% <ø> (ø)` | :arrow_up: |
| [transformers/modeling\_auto.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL21vZGVsaW5nX2F1dG8ucHk=) | `29.89% <ø> (-0.72%)` | :arrow_down: |
| [transformers/tests/tokenization\_gpt2\_test.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL3Rlc3RzL3Rva2VuaXphdGlvbl9ncHQyX3Rlc3QucHk=) | `97.43% <ø> (ø)` | :arrow_up: |
| [transformers/data/metrics/squad\_metrics.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL2RhdGEvbWV0cmljcy9zcXVhZF9tZXRyaWNzLnB5) | `0% <0%> (ø)` | |
| [transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL21vZGVsaW5nX3V0aWxzLnB5) | `90.86% <100%> (ø)` | :arrow_up: |
| [transformers/tokenization\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL3Rva2VuaXphdGlvbl91dGlscy5weQ==) | `91.02% <100%> (+0.55%)` | :arrow_up: |
| [transformers/tests/tokenization\_tests\_commons.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL3Rlc3RzL3Rva2VuaXphdGlvbl90ZXN0c19jb21tb25zLnB5) | `100% <100%> (ø)` | :arrow_up: |
| [transformers/tokenization\_xlm.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL3Rva2VuaXphdGlvbl94bG0ucHk=) | `83.46% <100%> (+0.13%)` | :arrow_up: |
| [transformers/data/\_\_init\_\_.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL2RhdGEvX19pbml0X18ucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [transformers/tokenization\_xlnet.py](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree#diff-dHJhbnNmb3JtZXJzL3Rva2VuaXphdGlvbl94bG5ldC5weQ==) | `90.4% <100%> (+0.15%)` | :arrow_up: |
| ... and [7 more](https://codecov.io/gh/huggingface/transformers/pull/2101/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/2101?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/transformers/pull/2101?src=pr&el=footer). Last update [0cb1638...35737ea](https://codecov.io/gh/huggingface/transformers/pull/2101?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Hi @mandubian, thanks for opening a PR to fix this. I think this raises an issue when there are two new tokens added which are right after each other, as spaces get added before and after the tokens. Here's an example of the issue, with your change and based on #2096:

```py
from transformers import BertTokenizer
bert_tokenizer = BertTokenizer.from_pretrained('bert-base-cased')

bert_tokenizer.add_tokens(['[ENT]', '[TEN]'])
print(len(bert_tokenizer))

x = bert_tokenizer.encode("you are the [ENT] [TEN] with [ENT] and [ENT]")

print(bert_tokenizer.decode(x))
# outputs: [CLS] you are the [ENT]  [TEN] with [ENT] and [ENT] [SEP]
# with two spaces ----------------^^
```

You're right, I hadn't thought about the case of 2 consecutive tokens. Let's try to make it better.

@LysandreJik I've pushed a new version for discussion.

Added tokens aren't prepended with space anymore but subtexts are. I've considered different solutions but none is perfect and a compromise has to be made. 

In Bert tokenizer, `convert_tokens_to_string` joins with space between sub-strings (not in GPT2 tokenizer) but then those sub-strings and added tokens need to be separated also by spaces. So I choose to remove the space before added tokens and add a space in subtexts join so that there are always spaces.

But it can add spaces where there weren't. With Bert Tokenizer, if you have `[ABC] toto tata [DEF] [GHI]`, `decode.encode` returns the same string (except lower case). But when you have less spaces `[ABC]toto tata [DEF][GHI]`, `decode.encode` returns `[ABC] toto tata [DEF] [GHI]` with more spaces.
For GPT2, it's the same, it doesn't respect all spaces from input.

I've added a test in `tokenizer_bert_test` and `tokenizer_gpt2_test` but it's not so good as it must be implemented for all tokenizers.

Don't hesitate to give more ideas, it's just a proposition on this quite stupid issue quite far from models 😄 

This seems to work, thanks @mandubian! I pushed another commit on your fork to test every tokenizer + rebase on master.

@LysandreJik (I've deleted my previous message from tonight, I had misread your message on mobile :D) Just to know, don't you squash commits in general?