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

Support Pipfile #3181

Open
gsemet opened this Issue Oct 23, 2017 · 36 comments

Comments

Projects
None yet
@gsemet

gsemet commented Oct 23, 2017

I prefer using Pipfile over requirements.txt. Please support this file format to set up the virtual environment.

@RichardLitt

This comment has been minimized.

Member

RichardLitt commented Oct 24, 2017

Thanks @stibbons. Contributions are always welcome, of course, if you'd like to submit a PR for this.

@agjohnson

This comment has been minimized.

Contributor

agjohnson commented Oct 24, 2017

I started playing with support for pipenv, it looks like it might be a drop in replacement for calls to pip install. We'll probably want to offer the option through our readthedocs.yml file, so this will take:

  • Conditional call to pipenv in doc_builder/python_environments.py
  • readthedocs.yml support in our readthedocs-build repo
  • readthedocs.yml support in this repo
@gsemet

This comment has been minimized.

gsemet commented Oct 24, 2017

yes, it is a straigh replacement of pip (actually, a wrapper), you do pipenv install thispackage and it installs the package in the virtualenv (using pew) and modify Pipfile accordingly. For ReadTheDocs, I suggest you do pipenv install --dev and ensure the lib is installed in the environment. I usually use -e . to inject my lib under developement into the virtualenv, but I am not sure if everybody does it

@tuukkamustonen

This comment has been minimized.

tuukkamustonen commented Oct 24, 2017

You should actually do pipenv install --dev --ignore-pipfile. Without --ignore-pipfile, pipenv will just seek to upgrade the packages in Pipfile and not really do deterministic build. See pypa/pipenv#954 (comment).

@gsemet

This comment has been minimized.

gsemet commented Oct 24, 2017

indeed

@agjohnson

This comment has been minimized.

Contributor

agjohnson commented Oct 24, 2017

I'm already using pipenv for other projects, so have some familiarity. I believe for our case, we actually want --system as well, to utilize our existing virtualenvs. We have special storage for this, so that venvs persist across builds.

@tuukkamustonen this confuses me a little, but I've only recently started with pipenv and pipfile. What does ignoring the pipfile mean for projects that don't check in their pipfile.lock? It doesn't seem checking in pipfile.lock is a requirement.

I suppose the most correct is check for pipfile.lock, if so, --ignore-pipfile, otherwise don't?

Some of these options will likely be added/allowed override through our readthedocs.yml, --dev is likely one of those.

In the case of a requirements.txt, we should be able to just use pipenv install instead of pip install

@gsemet

This comment has been minimized.

gsemet commented Oct 24, 2017

Both side is ok. When you freeze the requirements.txt (ex using pip-tools), it act like when you use the lock file.
When you only describe the version by range or without version, it act like Pipfile.
The lock file is specific to the machine, it does not have the markers (ex « python_version < 3 »), but both should work.
I would use ‘pipenv install —dev —system’, it is like ´pip install -r requirements.txt -r requirements-dev.txt’

@tuukkamustonen

This comment has been minimized.

tuukkamustonen commented Oct 25, 2017

@agjohnson I am new to pipenv as well, but as I understand it, if you run just pipenv install it will NOT follow your Pipfile.lock, but instead look into Pipfile, and upgrade any packages that you haven't locked into exact version, and then update Pipfile.lock. So it's like pip-compile and pip-sync (from pip-tools package) combined. I think it's a bit confusing.

I suppose the most correct is check for pipfile.lock, if so, --ignore-pipfile, otherwise don't?

Yeah, if we are after deterministic builds, then the lock-file needs to be followed and --ignore-pipfile given. I don't know how it would make sense otherwise. And then there is pipenv update, that uninstalls/installs packages like pip-sync but it doesn't have --ignore-pipfile option... hmm.

The lock file is specific to the machine, it does not have the markers (ex « python_version < 3 »), but both should work.

This is a good reminder. Neither pip-tools or pipenv (which uses pip-tools underneath) support "universal" lock files. So if you create the lockfile under python 3.6 on Windows and then run it on 3.4 on Linux, things may break, because lockfile expects 3.6/Windows environment. See pypa/pipenv#857 and jazzband/pip-tools#563.

What does ignoring the pipfile mean for projects that don't check in their pipfile.lock

Those projects won't get deterministic builds. It's ok, but should be documented. One of the main benefits with pipenv is to get deterministic builds (improvement over simple requirements.txt), but as you say, it's not a requirement. User should just acknowledge that.

In the case of a requirements.txt, we should be able to just use pipenv install instead of pip install

I don't think pipenv reads requirements.txt-style files. In case there is no Pipfile why not just resort to old behavior?

@agjohnson

This comment has been minimized.

Contributor

agjohnson commented Oct 25, 2017

This is a good reminder. Neither pip-tools or pipenv (which uses pip-tools underneath) support "universal" lock files. So if you create the lockfile under python 3.6 on Windows and then run it on 3.4 on Linux, things may break, because lockfile expects 3.6/Windows environment. See pypa/pipenv#857 and jazzband/pip-tools#563.

Ah yes, so it seems that ignoring the Pipfile and relying on Pipfile.lock should be an option, but is off by default. Without universal lock files, we can't guarantee that the environment is the same OS or even python version as our build images.

I don't think pipenv reads requirements.txt-style files. In case there is no Pipfile why not just resort to old behavior?

It looks like if a Pipfile is missing and a requirements.txt is available, pipenv install will automatically translate this to a Pipfile. Likewise, if the file is named foo.txt, pipenv install -r foo.txt will pick up the arbitrarily named file. These are the two modes we need to support on RTD.

@gsemet

This comment has been minimized.

gsemet commented Oct 25, 2017

It does not change from requirements.txt, it may or may not be reproductible, depending on the work of the programmer... I would not bother, simply use the --dev, because most of the time ReadTheDoc is triggered at the same time than travis.

Later when universal lock file will be settled this can be adapted to Rtd, but this is not as "important" than in the travis build (because the artefact is the doc, not the actual wheel), and we dont have this ability in the travis.

Or, simpler, you let the developer write its install line (pip install -r requirements.txt, or pipenv install --dev), it is the more versatile solution.

@sobolevn

This comment has been minimized.

sobolevn commented Jan 16, 2018

What's the status of this issue? Maybe there is something I can help with?
Thanks!

@DmytroLitvinov

This comment has been minimized.

DmytroLitvinov commented May 4, 2018

Hey guys.
Are there any news?

@agjohnson

This comment has been minimized.

Contributor

agjohnson commented Jun 1, 2018

No updates. Feel free to take this work on if you want.

@agjohnson agjohnson added this to the Admin UX milestone Jun 1, 2018

@Tobotimus

This comment has been minimized.

Tobotimus commented Jun 16, 2018

Hey guys, firstly, thanks for all the work you guys do on this project 😃

I've taken an interest in this issue, and I'd just like to note a few things down about how we expect people will use this feature, and which use cases we should accommodate for. People often get confused about how pipenv should be used, and thus I expect it will be difficult to please everyone with this feature.

I am going to use the information provided in this section of pipenv's documentation as the main point of reference. Accommodating for the use cases and guidelines listed there should satisfy most developers.

RTD's installation process

So currently, there are three stages to the installation process after the virtualenv is created:

  1. Installing RTD's core requirements
  2. Installing user requirements from a requirements.txt file with pip (optional stage)
  3. Installing the package itself with pip install . or python setup.py install --force (also optional)

How pipenv fits into these stages

Installation of pipenv itself should be done in the first stage. I think this should be conditional based on the YAML config.

Now, some (or perhaps even "most") usages of pipenv would actually combine stages two and three, as many projects have -e . or some equivalent specification in their Pipfile (and subsequently their Pipfile.lock, which would also contain sub-dependencies of the package).

However, as specified in the documentation linked above, Pipfiles are also capable of managing dependencies by themselves without relying on the project's install_requires. In this case, pipenv would be used to complete stage 2, and pip install . or python setup.py install for stage 3, if required.

The simplest and most malleable approach I can think of is doing pipenv install {opts} as an additional stage between 2 and 3. Since stages 2 and 3 are already optional, this gives the user lots of freedom for how they'd like to set up the environment, without confusing the user with mutually exclusive configuration options. The documentation would obviously need to be quite clear on how these various configuration options can be used together.

Arguments to pipenv install

I think in the interest of giving the user freedom and also future-proofing the configuration for any new pipenv install flags, we should simply allow the user to specify the flags they'd like to use as a string in the YAML, however we should have --system set as a persistent flag to ensure pipenv doesn't create another virtualenv. Using pipenv to manage virtual environments is unnecessary as we already have a lovely robust design and API for setting up virtual environments with virtualenv, and pipenv's virtual environment management is more of an end-user QoL feature.

I think --ignore-pipfile is unnecessary as the default behaviour is to install from Pipfile.lock.

However, in any case, I think we ought not to specify packages or requirements files as arguments to pipenv install, since that is used for adding dependencies to a Pipfile and not deterministically recreating a development environment.

Config layout

So given the above points, I think my approach would be to add these options to the YAML:

pipenv_install: True|False
pipenv_install_opts: OPTIONS

Edit: On second though perhaps having these options as sub-keys below a pipenv key would be preferable.

The reason I'm not including it in the python key is because I think it should be on the same level as the requirements_file key, which this is most similar to.

Summary of installation with pipenv

  1. Install RTD's core requirements with pip, including pipenv if pipenv_install is True
  2. Optionally install requirements from a requirements.txt file with pip
  3. Optionally do pipenv install --system {opts}
  4. Optionally do either pip install .[extras] or python setup.py install --force, or neither

Please let me know if you have a different view and/or point out improvements which could be made 😃

@stsewd

This comment has been minimized.

Member

stsewd commented Jun 18, 2018

@Tobotimus thanks, that's a great summary of the use case of pipenv, currently, we are doing a v2 of the configuration file https://github.com/orgs/rtfd/projects/2, I think this should be there.

@Tobotimus

This comment has been minimized.

Tobotimus commented Jun 19, 2018

@stsewd Ah I see! Great. Unfortunately that link is a 404 for me, and I can't see any projects on rtfd's org page, however I did find the YAML file completion milestone, which I'm happy to help out with when I get some time 😄 I am a new here so still somewhat stumbling around.

I've opened up #4254 as a patch to the old config, but I am more than happy to refactor it according to any new spec. Is there a feature branch of some sort for the new config? Thanks!

@stsewd

This comment has been minimized.

Member

stsewd commented Jun 19, 2018

We are moving the rtd-build repo to the readthedocs.org repo #4242, there are some ideas here https://docs.readthedocs.io/en/latest/design/yaml-file.html and this is kind of the base schema we have for the v2 https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/rtd_tests/fixtures/spec/v2/schema.yml. There are some design decisions to make.

@Tobotimus

This comment has been minimized.

Tobotimus commented Jun 19, 2018

Thanks very much, I suppose my approach from #4254 could easily adapt to that schema by simply placing the pipenv section under the python key. For reference, here is the patch I prepared for the rtd-build repo. I suppose I will wait for #4242 to be merged and for more progress to be made on the V2 config parser before continuing with pipenv PRs 😃

@agjohnson

This comment has been minimized.

Contributor

agjohnson commented Jun 19, 2018

There is more work to finish up the porting process, so it would probably be best to wait for that process to be completed first. I suppose we can consider readthedocs_build frozen for now.

Once things are ported, you can rework your PR to be housed here.

For now, a separate PR that updates our v2 schema spec file would be the best way to start. You'll find tests and the schema here:
https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/rtd_tests/fixtures/spec/v2/schema.yml
https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/rtd_tests/tests/test_build_config.py

This would be the best place to describe your changes and rationale for schema changes.

The immediate feedback I'd have is that I don't like exposing a pipenv.options, we don't do this for any other tools, we rather support specific options. I agree that this should all be under a python.pipenv key, but perhaps we also need to change python.install to support pipen instead of pip.

Edit: cc @Tobotimus, also 🎉 for the contribution! Sorry your PR landing right in the middle of a large project

@agjohnson agjohnson added Feature and removed Improvement labels Jun 19, 2018

@agjohnson agjohnson modified the milestones: Admin UX, 3.0 Jun 19, 2018

@Tobotimus

This comment has been minimized.

Tobotimus commented Jun 20, 2018

@agjohnson Thanks for the info! And also thanks for the feedback. I can understand that exposing pipenv.options is inconsistent with the rest of the schema.

I suppose in that case specific options we should support for now would just be --dev (install development dependencies). As for other options:

  • --skip-lock doesn't make any sense to me for people to use since it just makes the dependencies vague, defeating the purpose of the tool.
  • --ignore-pipfile we could add as persistent, it wouldn't make a difference 99% of the time since pipenv install by itself always installs from Pipfile.lock unless it's out of date with the Pipfile.
  • --system should remain persistent to ensure we don't create a new virtual environment.

Also I don't think python.install needs to be changed as in a significant fraction of cases, pipenv is not actually a replacement for pip, but a replacement for python.requirements, in which case some users would want both a pipenv install stage and a pip install . stage.

@agjohnson

This comment has been minimized.

Contributor

agjohnson commented Jun 20, 2018

Great, agreed. It sounds like --dev is the main one we should support for now.

When i started working on this, I just assumed we could treat pipenv as a drop in replacement for pip. Do you have any examples of packages that use a workflow with both a pip and pipenv step? I'm just curious as to what that actually looks like.

Adding a pipenv step, as opposed to replacing the pip step, isn't incorrect. Without knowing a lot about conda, perhaps that's also how we should treat conda as well.

@Tobotimus

This comment has been minimized.

Tobotimus commented Jun 20, 2018

In all honesty, I don't have an example of a package which uses that workflow, I just based my example use cases off this section of pipenv's documentation to be as unbiased as possible towards my own usage (which is to have pipenv install the project itself from the Pipfile.lock, thus being a replacement for pip). In particular, this workflow:

For applications, define dependencies and where to get them in the Pipfile and use this file to update the set of concrete dependencies in Pipfile.lock. This file defines a specific idempotent environment that is known to work for your project. The Pipfile.lock is your source of truth. The Pipfile is a convenience for you to create that lock-file, in that it allows you to still remain somewhat vague about the exact version of a dependency to be used.

That workflow does not mention using pipenv to install the package itself.

However, after a bit more research, perhaps this workflow is more angled towards projects which aren't actually pip installable. To quote the Python Packaging User Guide when they compare pipenv to poetry:

pipenv explicitly avoids making the assumption that the application being worked on that’s depending on components from PyPI will itself support distribution as a pip-installable Python package.

So you're probably right that treating pipenv as a replacement for pip would be sufficient 😃

@gsemet

This comment has been minimized.

gsemet commented Jun 20, 2018

Hi @agjohnson. I mainly use pipenv in all my python applications (and my librairies as well thanks to PBR). So far I very happy, and I have developed a small package that generate a requirements.txt so that readthedoc is happy even if it does not support Pipfile for the moment.

You can find a cookiecutter that bootstrap all of that for new projects: readthedocs compatible, pipenv, etc.

So if I could get rid of it if ReadTheDocs support seamlessly Pipfile (and pipenv), I would be very happy.

TD; DR: pipenv is primarily for application, not libraries. But readthedocs for app using pipenv would be great!

For sure, it should use the "--dev", I even think it should be by default. The question about the lock file is tricky, I do not have a clear opinion on it. I would let it optionable by the user:

  • using a lock file ensure the build is reproductible, this should be a requirements for both libraries and application
  • but for a library we should not lock the dependencies of the package, so that pip resolve the dependencies versions on pip install mydeps

My solution for the moment is to not track the lock file for libraries, but it makes my builds not reproductible. I'm not happy but I can live with it.

If you detect a Pipfile in the current project, just prepend all your pip install with pipenv run like for example: pipenv run pip install sphinx sphinx-rtd-theme. Do not try to use pipenv install, it is pointless and risky since it launches the resolver

@petersmithca

This comment has been minimized.

petersmithca commented Aug 29, 2018

Is there any update on this? Seems like it should be high on the radar? Really holding me back from using readthedocs as I don't want to hack the codebase, then not be able to get updates.

@stsewd

This comment has been minimized.

Member

stsewd commented Aug 29, 2018

@petersmithca this is on our radar, we are trying to wrap up the v2 of the configuration file first, and trying to understand better the workflow of projects using a pipfile. Any help/comments about the desired workflow are welcome.

@petersmithca

This comment has been minimized.

petersmithca commented Aug 29, 2018

Maybe misunderstanding, but essentially my workflow basically replaces creating of virtualenv and pip install -r requirements.txt with pipenv install and any activation of the environment with pipenv shell

@Peque

This comment has been minimized.

Peque commented Aug 29, 2018

@petersmithca Rather than pipenv install I would use pipenv sync --dev instead. I normally put dependencies related to documentation generation and tests under the "dev" section in the Pipfile. Also, sync will make sure the installed packages match those defined in the Pipfile.lock, although, if that file does not exist, then pipenv install --dev would make sense.

@petersmithca

This comment has been minimized.

petersmithca commented Aug 29, 2018

@gsemet

This comment has been minimized.

gsemet commented Aug 29, 2018

prefere pipenv install --ignore-pipfile --dev to force using the lock file.

@stsewd

This comment has been minimized.

Member

stsewd commented Aug 30, 2018

About using pipenv as a replacement of venv/virtualenv, actually, we can still use them.

From pipenv docs

By default, Pipenv tries to detect whether it is run inside a virtual environment, and reuses it if possible. This is usually the desired behavior, and enables the user to use any user-built environments with Pipenv.

So, probably we can treat pipenv as another installation method (like pip or setup.py)

@stsewd

This comment has been minimized.

Member

stsewd commented Oct 17, 2018

I'm testing pipenv now, I found this:

  • --ignore-pipfile will try to generate a lock file if one isn't found.
  • When running pipenv install, it will generate a lock file.
  • When running with --skip-lock it will install from pipfile, without generating a lock

If an error happens when generating the lock file, it doesn't install the dependencies (it may fail if there are incompatible dependencies).

So, people using un-pined dep (like libraries), may have this problem, should it be a problem to be fixed by the users? Or should we put both options? --ignore-pipfile and --skip-lock? (--ignore-pipfile will be on by default)

@stsewd

This comment has been minimized.

Member

stsewd commented Oct 18, 2018

Pipenv doesn't support specify a custom Pipfile (we plan to support installations from other than roo directory). So, I'm planning to put that as an environment variable, the command looks like this

 PIPENV_PIPFILE=./custom/path/Pipfile pipenv install --system --dev --ignore-pipfile --skip-lock 

--skip-lock and --ignore-pipfile are options on the yaml file, --ignore-pipfile is on by default. Also, --dev is an option and is off by default.

With this we install the dependencies in the same virtual env. I'm using a configuration like this for this project https://github.com/gsemet/cfgtree:

version: 2
sphinx:
  configuration: docs/conf.py
python:
  install:
    - pipfile: .
      skip_lock: true
      dev: true
    - path: .
      method: pip
@stsewd

This comment has been minimized.

Member

stsewd commented Oct 19, 2018

Hey folks, I'm testing the pipfile support on rtd, can you please comment with a project that is using a pipfile (and have docs to build)? That will help me a lot on testing and bring a less buggy implementation p:

@willrogers

This comment has been minimized.

willrogers commented Oct 29, 2018

https://github.com/dls-controls/pytac uses Pipenv, has a pipfile, but also has a rtd-requirements.txt file for this exact purpose. It's at https://pytac.readthedocs.io/en/latest/.

Thanks for looking at this!

@gsemet

This comment has been minimized.

gsemet commented Oct 29, 2018

That’s the role of pipenv-to-requirements/ :)

@stsewd

This comment has been minimized.

Member

stsewd commented Oct 30, 2018

@willrogers I was able to build your docs with the current implementation, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment