cleanup tf unittests: part 2
Author: stas00
Pull request: yes

This is part 2 of https://github.com/huggingface/transformers/pull/6196 and the original issue https://github.com/huggingface/transformers/issues/5973. This PR brings the tf and pt tests and templates to an almost identical template.

**edit: as other parts of code have evolved, the part of this comment below this line is no longer relevant - see the development of this PR in the subsequent comments**

---

As discussed at https://github.com/huggingface/transformers/issues/5973#issuecomment-668113798, a wrapper was needed to turn plain `results` dicts into object-like dicts, so that the `results.key` works the same whether it's returned by a class or it was just a local dict. I'm not sure `DictAttr` is the best name, but I had to start somewhere - it'll be an easy thing to rename it once you help me to find a better name. I did look for a pre-existing library to facilitate this, but didn't find anything that doesn't require installing another package.

To remind - the key change in part1 and in this clean up is to replace:

```
-            result = {
-                "sequence_output": sequence_output.numpy(),
-                "pooled_output": pooled_output.numpy(),
-            }
-            self.parent.assertListEqual(
-                list(result["sequence_output"].shape), [self.batch_size, self.seq_length, self.hidden_size]
-            )
-            self.parent.assertListEqual(list(result["pooled_output"].shape), [self.batch_size, self.hidden_size])
```
with:
```
+            result = DictAttr({"sequence_output": sequence_output.numpy(), "pooled_output": pooled_output.numpy()})
+            self.parent.assertEqual(result.sequence_output.shape, (self.batch_size, self.seq_length, self.hidden_size))
+            self.parent.assertEqual(result.pooled_output.shape, (self.batch_size, self.hidden_size))
```
like it was done in part 1, except part 1 didn't need the wrapper - `results` were bona fide objects there.

there is also one torch test in this batch that required the wrapper too: `test_modeling_transfo_xl.py`

@sshleifer

Comments:
Hmm, looks like `isort` and `flake` are in conflict with each other. `make style` reformatted the code to:
```
        result = DictAttr({"sequence_output": sequence_output.numpy(), "pooled_output": pooled_output.numpy(),})
```
and flake complains:
```
tests/test_modeling_tf_albert.py:139:110: E231 missing whitespace after ','
```
if I add the desired by `flake` whitespace, `isort` removes it.

How do I resolve such conflict?

**edit**: found a workaround, by removing the trailing comma:

```
perl -pi -e 's|\),\}\)|)})|' tests/test_modeling_tf* templates/adding_a_new_model/tests/test_modeling_tf_xxx.py
```



99% of this PR has been facilitated by running:

```
perl -0777 -pi -e 's#self.parent.assertListEqual\(
[\s\n]*
list\((result\w*)\[" ([^"]+) "\].(?:shape|size\(\))\),[\s\n]+\[ ( [^\]]* ) \],?
[\s\n]*
\)
#self.parent.assertEqual($1.$2.shape, ($3))#xmsg' tests/test*py templates/adding_a_new_model/tests/test_modeling_tf_xxx.py

perl -0777 -pi -e 's#
result\s=\s\{
([^}]+)
\}
#result = DictAttr({$1}) #xmsg' tests/test*py templates/adding_a_new_model/tests/test_modeling_tf_xxx.py

perl -pi -e 's|^(from transformers.testing_utils import .*?)$|$1, DictAttr|' tests/test_modeling_tf_albert.py tests/test_modeling_tf_bert.py tests/test_modeling_tf_ctrl.py tests/test_modeling_tf_distilbert.py tests/test_modeling_tf_electra.py tests/test_modeling_tf_flaubert.py tests/test_modeling_tf_gpt2.py tests/test_modeling_tf_mobilebert.py tests/test_modeling_tf_openai_gpt.py tests/test_modeling_tf_roberta.py tests/test_modeling_tf_t5.py tests/test_modeling_tf_transfo_xl.py tests/test_modeling_tf_xlm.py tests/test_modeling_tf_xlnet.py test_modeling_transfo_xl.py templates/adding_a_new_model/tests/test_modeling_tf_xxx.py

make style
# remove trailing comma that breaks flake
perl -pi -e 's|\),\}\)|)})|' tests/test_modeling_tf* templates/adding_a_new_model/tests/test_modeling_tf_xxx.py
```

# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/6260?src=pr&el=h1) Report
> Merging [#6260](https://codecov.io/gh/huggingface/transformers/pull/6260?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/0735def8e1200ed45a2c33a075bc1595b12ef56a&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

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

```diff
@@           Coverage Diff           @@
##           master    #6260   +/-   ##
=======================================
  Coverage   80.08%   80.08%           
=======================================
  Files         153      153           
  Lines       27984    27984           
=======================================
  Hits        22412    22412           
  Misses       5572     5572           
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/6260?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_tf\_distilbert.py](https://codecov.io/gh/huggingface/transformers/pull/6260/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9kaXN0aWxiZXJ0LnB5) | `64.47% <0.00%> (-32.95%)` | :arrow_down: |
| [src/transformers/file\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/6260/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9maWxlX3V0aWxzLnB5) | `82.44% <0.00%> (+0.25%)` | :arrow_up: |
| [src/transformers/modeling\_tf\_gpt2.py](https://codecov.io/gh/huggingface/transformers/pull/6260/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9ncHQyLnB5) | `94.72% <0.00%> (+22.87%)` | :arrow_up: |
| [src/transformers/tokenization\_albert.py](https://codecov.io/gh/huggingface/transformers/pull/6260/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fYWxiZXJ0LnB5) | `87.50% <0.00%> (+58.65%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/6260?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/6260?src=pr&el=footer). Last update [0735def...d3380ba](https://codecov.io/gh/huggingface/transformers/pull/6260?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Hi @stas00. I'm in the process of changing all model outputs of TF models to the same as PyTorch and updated all the tests. This is in #6247 that I expect to merge today (once merge conflicts are handled and I've added the templates).

Could you wait a tiny bit and then apply the same PERL commands you did in #6196 ?

Oh, that's even better - yes, of course, I will wait, @sgugger! If you remember please ping me when this is done.  Thank you! 

Could you also check on this one?
https://github.com/huggingface/transformers/blob/master/tests/test_modeling_transfo_xl.py#L94
It's the only remaining torch test with `results` dict.

For the slow test, we can use `outputs1["last_hidden_state"]` and so forth in the asserts after. I went for the quickest fixes in the tests.

> For the slow test, we can use `outputs1["last_hidden_state"]` and so forth in the asserts after. I went for the quickest fixes in the tests.

Part of this rewrite was replacing `results["key"]` with `results.key` as specified. So this one would be different. There are two constructors in `test_modeling_transfo_xl.py` that do that. All other tests have or will follow the latter style once you finish your work and I will update mine. 

The outputs PR has been merged, so pinging you @stas00 

I will rework this PR to match @sgugger changes.

@sgugger, after your merge, these 3 tf tests are still using the old style it seems:
```
FAILED tests/test_modeling_tf_transfo_xl.py::TFTransfoXLModelTest::test_transfo_xl_lm_head - AttributeError: 'dict' object has no attribute 'lm_logits_1'
FAILED tests/test_modeling_tf_transfo_xl.py::TFTransfoXLModelTest::test_transfo_xl_model - AttributeError: 'dict' object has no attribute 'hidden_states_1'
FAILED tests/test_modeling_tf_xlnet.py::TFXLNetModelTest::test_xlnet_lm_head - AttributeError: 'dict' object has no attribute 'all_logits_1'
```
I reverted to the `["key"]` style for these for now.

> I guess we can deal with the 4 remaining tests (one PT, 3 TF) in a separate PR.

Yes. Could you please instruct me on how to proceed with those or will you take care of them?

I think just using the result1/result2 in the assers instead of grouping everything in a dict should be fine.

please re-run CI - unrelated failure - thank you.

If there is any more cleanup you'd like to see send me the before and after code snippets.