RFC -- TF: unpack model inputs through a decorator
Author: gante
Pull request: no

# RFC

This is an RFC concerning how model/layer inputs are handled in TF. The details are below and it includes a working demo with the proposed solution -- comments (and concerns) are deeply appreciated.

cc @Rocketknight1 @LysandreJik @sgugger 


## Motivation

In all TF models and main layers, in their `call` function, we start by processing their inputs with `input_processing()` ([example](https://github.com/huggingface/transformers/blob/master/src/transformers/models/bert/modeling_tf_bert.py#L741)). This function is needed to handle multiple input formats, from a dictionary of tensors packed in the first argument of `call` to variables of diverse types passed through keyword arguments (or a combination of both), as well as to handle some control logic. 

The `input_processing()` function outputs a dictionary containing the processed function arguments, often held in the `inputs` variable. Sadly, it forces us to access all function arguments through the `input` dictionary throughout its corpus, which results in verbose and less clear code ([example](https://github.com/huggingface/transformers/blob/master/src/transformers/models/bert/modeling_tf_bert.py#L815)). Furthermore, it is not beginner-friendly -- it's not obvious at first how, why, and when we need to use the `input` dictionary, as opposed to the function arguments.


## Proposed solution

The `inputs` dictionary is essentially a dictionary containing the function arguments and keyword arguments (plus potentially extra `**kwargs` that were unpacked in the process). If we were to call `call` again using these processed inputs, no input processing should be needed. The proposed solution is precisely that -- a decorator that runs `input_processing()` with whatever is passed to the function, sending its output to `call`.

As a proof of concept, I've built the decorator and applied it to a single class in [this draft PR](https://github.com/huggingface/transformers/pull/15907) -- all associated tests pass. As you can see, the resulting code is shorter, clearer, and makes use of the expected function arguments throughout its corpus. It is also backward compatible.

Cons: type hinting and decorators don't go along very well, if we were to rely more heavily on type hinting.


## Proposed plan for adoption

If we agree to go forward with this solution, here's my proposed plan:
1. Open a PR with the decorator, including its application on an NLP model and some other non-NLP model. This would confirm that the decorator works fine for multiple modalities;
2. Applying the decorator is straightforward, so we could open an issue with the `Good First Issue` label. I'd keep track of collaborators, and give it a deadline (e.g. 1 month) after which I'd take over remaining models;
3. After all models are updated, move `input_processing()` so as to be an internal function of the decorator.

Comments:
This is a really good idea, I think! The loss of type hinting might be an issue, though - I think our models are almost totally type-hinted in PT, though that's probably just to get them to compile with Torchscript and isn't strictly necessary for TF.

Probably there's some way to salvage this, though - Py3.10 added [PEP 612](https://www.python.org/dev/peps/pep-0612/) which specifically addresses this. Unfortunately, we can't force people to update to Py3.10 quite yet, but maybe there's a path where we start with your proposal and switch to this one once it's acceptable to make Py3.10 the minimum version?

I do agree that every `call()` immediately calling `input_processing()` is currently quite ugly and confusing for newcomers, and some kind of fix would be extremely welcome, though.

The type hints are necessary for the documentation (see for instance [TFBertModel.call](https://huggingface.co/docs/transformers/model_doc/bert#transformers.TFBertModel.call) and the nice feature where you can hover) so I would be really sad to see them disappear. We also need to keep the docstring (for obvious reasons) and diverse attributes like `__module__`, `__name__`, `__qualname__` for the doc building.

The good news is, I believe you can fix this some [`functools.wrap`](https://docs.python.org/3.6/library/functools.html#functools.wraps) dark magic :-)

@sgugger if the live docs are to be trusted, then the proposed decorator requires no doc-related changes 🎊  (but maybe I'm missing something 🤔 )

In the [draft PR](https://github.com/huggingface/transformers/pull/15907), I've applied the decorator to `TFBertModel.call`, and waited for the doc CI to run. The docs had no changes: [PR docs for TFBertModel.call](https://moon-ci-docs.huggingface.co/docs/transformers/pr_15907/en/model_doc/bert#transformers.TFBertModel.call) / [master docs for TFBertModel.call](https://huggingface.co/docs/transformers/model_doc/bert#transformers.TFBertModel.call)

So there is no problem with type hints then :-) The doc-builder relies on the signature for the formatting of params, so if it's rendered normally it means the signature did not change.

I can't believe I didn't begin my first comment with it, so belatedly: it's a great idea! I love how elegant it makes the code!

Just played with it locally and I don't see any change in IDE, so definitely go for this! You can polish the PR to make it not-draft and your plan seems a good one afterward. No need to wait for @LysandreJik as I'm pretty confident he'll love it as much as I do :-) (and he can review the cleaned PR when he's back).