build: add poetry, an alternative to setup.py with dependency versions tracked
Author: aurelien-clu
Pull request: yes
Labels: name: wontfix

Hello,
I have written a `pyproject.toml` so your project can be setup using [poetry](https://github.com/python-poetry/poetry) .

That way requirements versions can easily be tracked for people wanting to use **poetry** (it is optional).

# Example

```bash
# first setup your virtual environment, then:
pip install poetry
poetry install  # this is equivalent to 'pip install .' but with versions tracked
poetry install --extras testing # pip install -e ".[testing]"
poetry install --extras examples # pip install -r examples/requirements.txt
poetry install --extras torch  # pip install -e ".[torch]"
poetry install --extras tf  # pip install -e ".[tf]"

# edit: updating dependencies to the latest possible:
poetry update

# adding new dependencies
poetry add MyPyModule
```

# Notes

This does not change any python code, i.e. everything still works :)


Comments:
Not sure why this breaks the CI?

Shouldn't we _not_ version control the .lock file?

To me one interesing is to track `.lock` that way you are always certain to have a working versions, i.e. dependencies version that match together.

Indeed I am not sure why the CI fails :/

Pinging @aaugustin our Python-ecosystem/packaging expert on this, but I don’t think we want to commit to maintaining multiple different install systems

https://circleci.com/gh/huggingface/transformers/15309?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
```
builder = WheelBuilder(poetry, SystemEnv(Path(sys.prefix)), NullIO())
```

The CI now uses `poetry` for `wheels` which was unexpected to me :/


`poetry build -f wheel` broken for C extensions
#1332 : https://github.com/python-poetry/poetry/issues/1332

I am not for changing the way you build, but only give the option to users to be able to manage dependencies using `poetry` :wink: 


I have squashed my commits (sorry for the multiple CI runs)

Some issues where:

- `python 3.5` is needed (I used 3.6 so it complies with `black`), so its matches your `setup.py`
- email address of one authors was not compliant (needs to be: `"author <email>"`)

New error:

```
The following workers failed to return coverage data, ensure that pytest-cov is installed on these workers.
```

I am investigating.

Edit:

Could it be because I push once more?
Because it is present in `.circleci/config.yml` and seems to be installed during the `ci` :thinking: 
And it works for few tests having also `pytest-cov`

Edit 2: **all good**, must have been committing once more

# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/2760?src=pr&el=h1) Report
> Merging [#2760](https://codecov.io/gh/huggingface/transformers/pull/2760?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/33d3072e1c54bcd235447b98c6dea1b4cb71234c?src=pr&el=desc) will **decrease** coverage by `25.36%`.
> The diff coverage is `n/a`.

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

```diff
@@             Coverage Diff             @@
##           master    #2760       +/-   ##
===========================================
- Coverage    75.1%   49.73%   -25.37%     
===========================================
  Files          93       93               
  Lines       15249    15249               
===========================================
- Hits        11452     7584     -3868     
- Misses       3797     7665     +3868
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/2760?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_tf\_camembert.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9jYW1lbWJlcnQucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_xlm\_roberta.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl94bG1fcm9iZXJ0YS5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_roberta.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9yb2JlcnRhLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_ctrl.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9jdHJsLnB5) | `0% <0%> (-97.83%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_t5.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl90NS5weQ==) | `0% <0%> (-96.55%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_bert.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9iZXJ0LnB5) | `0% <0%> (-96.06%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_openai.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9vcGVuYWkucHk=) | `0% <0%> (-95.85%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_distilbert.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9kaXN0aWxiZXJ0LnB5) | `0% <0%> (-95.13%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_gpt2.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9ncHQyLnB5) | `0% <0%> (-94.67%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl91dGlscy5weQ==) | `0% <0%> (-92.83%)` | :arrow_down: |
| ... and [18 more](https://codecov.io/gh/huggingface/transformers/pull/2760/diff?src=pr&el=tree-more) | |

------

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


Like @julien-c said, we don't want to maintain both poetry and setuptools configurations, because this is likely to create confusion and waste everyone's time (which already started with CI in this PR).

Switching to poetry could be a good move, but then we should ditch `setup.py` entirely and make sure all workflows are still operational.

> Like @julien-c said, we don't want to maintain both poetry and setuptools configurations, because this is likely to create confusion and waste everyone's time (which already started with CI in this PR).
> 
> Switching to poetry could be a good move, but then we should ditch `setup.py` entirely and make sure all workflows are still operational.

I am not against you ditch `setup.py` but that's your call :wink: 

As for not wanting to maintain both `poetry` and `setuptools`, maybe there will be people wanting to use `poetry` and will maintain it themselves. This would not mean any extra work for people not wanting to update `poetry` (even if I think maintaining `poetry` does not require much effort, there was some at the beginning and it has been done :wink: )


Replying to the discussion about the lock file: in pipenv projects I never share the lock file. Yes, you get better (locked) version control but in practice this does not work cross platform at all. Hashes for slightly more complex packages are mostly platform dependent. Installations between colleagues failed because of this. The lock file is a good idea in practice or for in-house deployment but is not useful in the real world, I think. 

On the lock file discussion, I think it's not worth it to version it in git for libraries, in general. The pro is an almost reproducible environment. Then con is having to constantly keep it up-to-date for new versions of all the dependencies (including the transitive ones), even for little changes (e.g., tqdm from 4.36.0 to 4.36.1). You could also avoid updating it, but then you'd never catch bugs on new versions. So I think it's good to keep reproducibility on python projects that are not libraries, especially when you want to make sure your code works on production as similar to your env as possible.

As an outsider, I see moving to poetry as a good idea. Pros: it works well and fast, specifying the test/dev/docs dependencies, simpler and fewer package configuration files (in theory, only `pyproject.toml`), can check if your env complies with the config file, can specify Python versions, can publish the package easily, more flexibility when specifying the dependencies' versions. The only con I see, apart from learning the tool which should be fast, is that `pip install --editable` wouldn't work as of today for the users.

> in practice this does not work cross platform at all

I agree that's an argument against it.

> Then con is having to constantly keep it up-to-date for new versions of all the dependencies (including the transitive ones), even for little changes (e.g., tqdm from 4.36.0 to 4.36.1).

I see no reason why you would need to keep it up-to-date. To me it is simply a (near) guarantee to be able to have a working environment to develop on the project. No matter if you don't have all the latest updates. Most little changes from dependencies have little to no impact on your own development. (library or project)

Anyhow, feel free to tell me to remove the `.lock` or to close this issue & PR 😉 


> I see no reason why you would need to keep it up-to-date. To me it is simply a (near) guarantee to be able to have a working environment to develop on the project. No matter if you don't have all the latest updates. Most little changes from dependencies have little to no impact on your own development. (library or project)

The problem I see is that some dependency versions are gonna stall forever, while actually the latest ones haven't been tried and are more likely to break the codebase.

> The problem I see is that some dependency versions are gonna stall forever, while actually the latest ones haven't been tried and are more likely to break the codebase.

It does not seem to be a good behavior to add breaking new dependencies 🤔 (especially in a `lib` with 22k ⭐️ )

As for stalling ones, `poetry update` most of the time will do the trick (updating your state to a newer working state) and I suppose there should be 1 or several people interested with having a most up to date setup and could contribute it though I may be naive 😇 


> It does not seem to be a good behavior to add breaking new dependencies  (especially in a `lib` with 22k  )

Not breaking dependencies, but a dependency version update makes your codebase to break, especially when you have an old version because the one in the codebase is quite old.

> As for stalling ones, `poetry update` most of the time will do the trick (updating your state to a newer working state) and I suppose there should be 1 or several people interested with having a most up to date setup and could contribute it though I may be naive

There's dependabot. But I think it's not worth it, that's my point.

A compromise could be that dependabot sends PRs monthly so it's less overwhelming. But I still don't see keeping the test env reproducible as an advantage (it doesn't reflect users' env).

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.