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

Added cross-environment usage documentation to README.rst #651

Merged
merged 1 commit into from
Mar 28, 2020

Conversation

vphilippon
Copy link
Member

@vphilippon vphilippon commented Apr 16, 2018

Added some documentation regarding cross-environment usage of requirements.txt.
In short: Run pip-compile on each targeted environment.

Resolves #585
Resolves #563

Changelog-friendly one-liner: Add cross-environment usage documentation to README.

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@vphilippon
Copy link
Member Author

@tuukkamustonen @barrywhart @cancan101 @taion @JoergRittinger
FYI as you've been the ones discussing mostly discussing the cross-environment topic.
Do tell me if something isn't clear or if you have any questions!

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@taion
Copy link

taion commented Apr 16, 2018

Yeah, giving Python packaging limitations this does make sense.

This is incompatible with checking in requirements.txt, though, right?

@vphilippon
Copy link
Member Author

vphilippon commented Apr 16, 2018

@taion Good point about source control. That depends on the user's case I guess. If they deploy an app/service through CI/CD on a single defined Python environment (ex: Only Py3.6 on linux), then committing requirements.txt to source control makes sense. If they deploy on multiple environments from a single repo, then the requirements.txt shouldn't be commited to source control.

I'll figure a way to point that out w/o having to write a dissertation on CI/CD.

@taion
Copy link

taion commented Apr 16, 2018

It's tricky, though. For example, we mostly develop on OS X, but our CI runs in a Linux Docker container.

@vphilippon
Copy link
Member Author

@taion Argh, yeah I see. If you don't mind me asking: what are you currently doing to deal with that?

@taion
Copy link

taion commented Apr 16, 2018

Nothing – just running pip compile on OS X gives good-enough results. Not perfect, but even when there are mismatches, it’s good enough. This may well be due to specific pecularities with our setup, though (we now get tensorflow-gpu from the Docker image whenever we run on Linux, so we never need to put it in our requirements files).

@vphilippon
Copy link
Member Author

(Pushed but I still need to take care of the source control recommendations)

@davidovich
Copy link
Contributor

@vphilippon

If they deploy on multiple environments from a single repo, then the requirements.txt shouldn't be commited to source control.

I'm not sure you should cristallise in doc not commiting requirements.txt to source control. Not doing so opens the door to non deterministic builds (because a compile phase is needed to re-create).

If you want to talk about this, I would instead mention that the files on different platforms should have different annotation (a platform suffix) corresponding to the source environment. That way you keep deterministic behavior for the different platforms.

@taion
Copy link

taion commented Apr 17, 2018

To be more concrete, now that I'm at a laptop, in the one repo where this even comes up for us, we do:

tensorflow; 'linux' not in sys_platform

On Linux we use a Docker image with TensorFlow pre-installed.

It so happens to work, but it's obviously far from perfect.

In practice, though, with the frequency with which people develop on OS X but deploy on Linux, it's not clear to me that the recommendation here taken literally would improve DX.

Really this is the fault of Python packaging; it's not clear there's much that's possible here

@JoergRittinger
Copy link
Contributor

Looks good to me.

@vphilippon
Copy link
Member Author

@davidovich @tuukkamustonen @taion Updated, with a distinct section regarding commiting.

README.rst Outdated Show resolved Hide resolved
Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish this PR had been merged some day. Could be very useful.

README.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase

@bhrutledge
Copy link
Contributor

@vphilippon This was a handy read. Are you game to rebase so that this can be merged? If not, should somebody else open a new PR?

I'd like to confirm that the recommendation is to generate and commit a requirements file for each environment. For example, I'm currently in the process of migrating a Django application from Python 2 to Python 3. We have base and dev requirements. Based on this PR, here's a tox configuration that I came up with:

[tox]
envlist = {2.7,3.7}
skipsdist = true

[testenv]
basepython =
    2.7: python2.7
    3.7: python3.7
envdir =
    {toxworkdir}/requirements-{envname}
deps =
    pip-tools
commands =
    pip-compile -o requirements-{envname}.txt \
        requirements.in
    pip-compile -o requirements-{envname}-dev.txt \
        requirements-{envname}.txt requirements-dev.in

Resulting in these files:

requirements.in
requirements-dev.in
requirements-2.7.txt
requirements-2.7-dev.txt
requirements-3.7.txt
requirements-3.7-dev.txt

If I understand correctly, I should commit all of those files?

@bhrutledge
Copy link
Contributor

For anyone interested, I put together an example repo of how to manage cross-environment requirements files using Tox or Nox: https://github.com/bhrutledge/requirements-py2-py3

Feedback welcome.

@atugushev atugushev force-pushed the document-cross-environment-usage branch from 24fd03a to 2b6892f Compare March 28, 2020 16:40
Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 I've rebased and squashed commits.

@atugushev atugushev added this to the 5.0.0 milestone Mar 28, 2020
@atugushev atugushev requested a review from auvipy March 28, 2020 16:44
@atugushev
Copy link
Member

@auvipy

please rebase

Addressed in 2b6892f.

@atugushev atugushev removed the needs rebase Need to rebase or merge label Mar 28, 2020
@codecov
Copy link

codecov bot commented Mar 28, 2020

Codecov Report

Merging #651 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #651   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files          34       34           
  Lines        2443     2443           
  Branches      302      302           
=======================================
  Hits         2429     2429           
  Misses          8        8           
  Partials        6        6           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4a4f7b...2b6892f. Read the comment docs.

@auvipy auvipy merged commit 78b5fc1 into jazzband:master Mar 28, 2020
@atugushev
Copy link
Member

Thanks to @vphilippon and all reviewers!

anthrotype added a commit to googlefonts/picosvg that referenced this pull request Nov 27, 2020
... parametrized by python version.

We have a bunch of dependencies that are only needed on specific python
versions (e.g. backports like dataclasses, importlib_resources, etc).
We use conditional environment markers for those in the top-level
requirements.in files.
pip-compile does not yet support generating a combined requirements.txt
file where differences across python versions/platforms/archs are
reconciled using environmet markers:
jazzband/pip-tools#826

They currently recommend running pip-compile on each targeted python
environment generating as many concrete requirements.txt files
jazzband/pip-tools#651

So this is what I have done here.
anthrotype added a commit to googlefonts/picosvg that referenced this pull request Nov 27, 2020
... containing all top-level (*.in) and concrete requirements.txt, the
latter parametrized by python version.

We have a bunch of dependencies that are only needed on specific python
versions (e.g. backports like dataclasses, importlib_resources, etc).
We use conditional environment markers for those in the top-level
requirements.in files.
pip-compile does not yet support generating a combined requirements.txt
file where differences across python versions/platforms/archs are
reconciled using environmet markers:
jazzband/pip-tools#826

They currently recommend running pip-compile on each targeted python
environment generating as many concrete requirements.txt files
jazzband/pip-tools#651

So this is what I have done here.
anthrotype added a commit to googlefonts/picosvg that referenced this pull request Nov 30, 2020
... containing all top-level (*.in) and concrete requirements.txt, the
latter parametrized by python version.

We have a bunch of dependencies that are only needed on specific python
versions (e.g. backports like dataclasses, importlib_resources, etc).
We use conditional environment markers for those in the top-level
requirements.in files.
pip-compile does not yet support generating a combined requirements.txt
file where differences across python versions/platforms/archs are
reconciled using environmet markers:
jazzband/pip-tools#826

They currently recommend running pip-compile on each targeted python
environment generating as many concrete requirements.txt files
jazzband/pip-tools#651

So this is what I have done here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to Cross Compile Environment markers do not carry out to transitive dependencies
8 participants