pypa / pipenv

Python Development Workflow for Humans.
https://pipenv.pypa.io
MIT License
24.82k stars 1.86k forks source link

Allow Pipfile to set VENV_IN_PROJECT #5747

Open justin-yan opened 1 year ago

justin-yan commented 1 year ago

There are a few similar issues, and I think https://github.com/pypa/pipenv/issues/2086 is probably the closest. I considered making a comment there, but thought my request was different enough that it maybe warranted its own top-level issue. If you feel otherwise, please do merge it with another issue!

Is your feature request related to a problem? Please describe.

In my codebase, I provide a justfile (https://just.systems/man/en/) to provide some standard commands, some examples (allows teammates to run just typecheck, just test, and just format to execute some standard local dev workflows)

@typecheck:
    pipenv run mypy --namespace-packages --explicit-package-bases -p {{NAMESPACE}}.{{PACKAGE_NAME}}
    pipenv run mypy --allow-untyped-defs --namespace-packages tests

@test:
    pipenv run pytest --hypothesis-show-statistics {{SRC_FOLDER}} {{TEST_FOLDER}}

@format:
    pipenv run ruff check --fix {{SRC_FOLDER}} {{TEST_FOLDER}}
    pipenv run black --verbose {{SRC_FOLDER}} {{TEST_FOLDER}}

One of the things that I try to support is that not only should the commands work locally if you have pipenv (and the packages from the Pipfile installed), which typically involves running pipenv sync --dev, but you should also be able to easily run it in a docker container:

@docker SUBCOMMAND:
    docker run -i -v `pwd`:`pwd` -w `pwd` {{DEV_IMAGE}} just {{SUBCOMMAND}}

What I'm trying to accomplish is to have the pipenv virtualenv installed in the folder, so that it can be mounted into the docker container, which helps me avoid two things:

  1. either reinstalling all my dependencies every time I run a command in docker
  2. having to bake new dev images every time I change my dependencies

Currently, I accomplish this by including the following command:

@init:
    [ -f Pipfile.lock ] && echo "Lockfile already exists" || pipenv lock
    PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev

and this works fairly well overall! The downside here is that if a teammate runs pipenv sync --dev directly instead of just init, they'll get their VENV directory in their home (or however they've configured their own pipenv install), and I'd like to enforce that the VENV for this project get installed directly in the project itself.

Describe the solution you'd like

My thought was that the downside of options like the flag (https://github.com/pypa/pipenv/issues/2086) or using env variables (PIPENV_VENV_IN_PROJECT) are that I still have to rely on my teammates to execute a specific command in order for the provided integration with docker to work, and if they run the very natural pipenv shell or pipenv sync --dev commands instead of the just init command that I provide, then their venv might end up in the wrong place.

My thought was that it would be nice if perhaps the Pipfile itself had a directive where I could specify pipenv_venv_in_project=true as a directive in the Pipfile, which would help ensure that anyone who uses the natural pipenv commands would "do the right thing" on a project-by-project basis.

One of the reasons I like pipenv when compared to other tools is that I can specify things like python version (and have it integrate with pyenv), which allows me to use the Pipfile as a more complete description of the entire "python environment", so hopefully this idea would be reasonably consistent with that.

justin-yan commented 1 year ago

Also - if this sounds like something the pipenv project would be willing to support, I'd be more than happy to take a crack at opening a PR for it, but wanted to run it by the team first.

matteius commented 1 year ago

I'm not totally having a directive in the pipenv section to venv_in_project=true -- I think having it named pipenv_venv_in_project is redundant since it would live in the [pipenv] directives for folks that want to enforce this at a project level, but I do feel like its kind of a matter of personal preference where the user has the virtualenv. Couldn't you also do this to get the virtualenv location and use what ever is provided?

$ pipenv --venv
Loading .env environment variables...
C:\Users\matte\.virtualenvs\pipenv_win
oz123 commented 1 year ago

The title of the issue is quite different from what you ask in the description. I am in favor of adding the possibility of having:

venv_in_project=true

Reading the issue title I felt like you want to achieve a random location. e.g:

venv_path=/some/random/path.

I would oppose such option as it provide a hatch for a bad UI for teams working with different OS.

Do you mind updating the title of the issue?

justin-yan commented 1 year ago

@matteius - That's definitely a fair perspective, it's certainly possible to accomplish what I'm doing by dynamically looking up the path and mounting it with docker. I feel like in the organizational setting we benefit from enforcing a convention (if someone's docker cmds don't work, I don't have to ask them where they've put their VENV, people modifying the justfile commands don't have to remember to handle dynamic path lookups, etc.), and having the venv live in the project allows us to use only one mount command.

I'd understand if you don't want to add the additional complexity - LMK what you think!

My apologies @oz123 ! When I first created the issue I was actually thinking of the full random location, but then after writing out more of use case details, I realized mostly what I wanted was being able to set the VENV in the project and then forgot to update the issue title. Updated it now!

I took a quick look and I think the change wouldn't be too intrusive? Likely just coalescing the env variable with a Pipfile directive in this method (and deciding on a precedence/what the behavior should be if there's disagreement?): https://github.com/pypa/pipenv/blob/98bdb5f8b2f08a435a825915d7d7d215a7aaec19/pipenv/project.py#L246

If this sounds reasonable, LMK and I'd be happy to take a crack at opening a PR for it. Thanks!

matteius commented 1 year ago

@justin-yan I don't have terribly strong opinions about this other than my own personal one which is that I sometimes will have multiple virtualenvs -- say I am testing new requirements updates or some other thing, having the Pipfile try to force me into using just one could create problems of its own for a project. However, since I could just activate a different virtualenv anyway and then pipenv will use that regardless of what you specify, it allows me to work around my concern, but then I wonder if its really going to accomplish what you want if people like me just activate a different virtualenv anyway. That's my two cents, but if you want to support it anyway be my guest. I would expect the change to include tests, doc updates (would be nice to call out in the docs that it is preferred to leave this to individual developers to decide but for some projects it may make sense to use this setting project wide.

justin-yan commented 1 year ago

I created a PR in my fork in order to solicit some feedback!

https://github.com/justin-yan/pipenv/pull/1

I'd definitely clean everything up/add docs/tests (and re-open the PR without a messy commit history) for a PR to the main repo, but wanted to put something concrete up to look at, since I think there are one or two questions that feedback would be useful for.