Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tell pip to install from setup.py #14

Conversation

nostalgebraist
Copy link
Contributor

Forces pip install -r requirements.txt to install the same package versions specified in setup.py.

For details, see this comment.

Confirmed that tests pass locally after merging this and #13 . (Since #13 fixes tests, they won't pass until it is merged.)

@jalammar
Copy link
Owner

The thing is, setup.py and requirements.txt are meant to be slightly different. setup.py is supposed to be loose so as not to not needlessly change the end-user's environment too much. Requirements.txt is a lot more specific in the the pinned versions it requires so as to replicate the development environment as much as possible:

Whereas install_requires requirements are minimal, requirements files often contain an exhaustive listing of pinned versions for the purpose of achieving repeatable installations of a complete environment.

In addition to that, in our specific case, requirements.txt requires pytorch, while setup does not -- mainly because it could get a little too involved (CPU vs. GPU support). So we leave it up to the user to have the flavor of pytorch they choose to be installed beforehand.

Does this make sense?

@nostalgebraist
Copy link
Contributor Author

nostalgebraist commented Dec 26, 2020

Does this make sense?

Sure. Sorry, this PR was jumping to a solution when I haven't clearly explained the problem I want to solve.

The problem is basically just the version pin transformers~=3.1.0 in requirements.txt:

  • A user installing via setup.py is not likely to have transformers 3.1.0 after the install.
    • That will only be true if they already had transformers installed and its version was coincidentally 3.1.0.
  • Getting transformers 3.4.0 is a much more typical result of installing via setup.py.
    • This will happen if the user did not already have transformers installed, or if they did, but its version was higher than 3.4.0

Heuristic: if I'm trying to make a reproducible development/test environment, I prefer to pin the versions to the latest versions allowed by setup.py, like 3.4.0 here. This makes my environment as close as possible to a typical user's.


Because of this heuristic, I'd prefer to pin transformers 3.4.0 instead of 3.1.0.

(This PR was trying to automatically apply the heuristic to all packages in install_requires, which is probably needless when our install_requires is very small. I have found this approach helpful in developing pakages with longer install_requires that frequently change, where the heuristic is tedious to follow otherwise.)

@jalammar
Copy link
Owner

That's perfectly reasonable. I'd be happy to update requirements.txt to transformers 3.4.0 and keep an eye on this in the future. Thanks for the heads up!

Would that resolve this?

@nostalgebraist
Copy link
Contributor Author

Would that resolve this?

It would, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants