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

Add env command #278

Closed
wants to merge 3 commits into from
Closed

Add env command #278

wants to merge 3 commits into from

Conversation

cauebs
Copy link
Contributor

@cauebs cauebs commented Jul 5, 2018

To satisfy the needs expressed in #270 (and, to some extent, #198), I added an env command.

~/foo $ pt env
Virtualenv path: /home/cauebs/foo/.venv
Python version: 3.6.5
Implementation: CPython

~/foo $ pt env --python
/home/cauebs/foo/.venv/bin/python

~/foo $ pt env --pip
/home/cauebs/foo/.venv/bin/pip

~/foo $ pt env --path
/home/cauebs/foo/.venv

~/foo $ pt env --rm
Removing virtual environment at /home/cauebs/foo/.venv

If this gets a greenlight, I'll add tests and docs.

  • Added tests for changed code.
  • Updated documentation for changed code.

@sdispater
Copy link
Member

Thanks for taking the time to put this PR together.

Some information are already present in the "debug:info" command. So I'm not sure about having duplicate commands.

Also, I am not a big fan of options executing actions. I know this is the pipenv way but I find it quite counter-intuitive. I prefer having command namespace (like the debug one). That way we could have a env:remove command.

Finally, I am still on the fence here about exposing the virtualenv since, to me, this is an implementation detail and, ideally, I would like Poetry to not even use them and use a more clever approach. I am not saying it will come anytime soon but introducing a env command will tie up Poetry to the virtualenv way.

Anyway, I am leaving this PR open for now since I still need to wrap my head around it and to continue the discussion.

@cauebs
Copy link
Contributor Author

cauebs commented Jul 9, 2018

That's reasonable enough. Don't feel bad about closing this PR, it was just an experiment. But I do think #270 requires some action, and that feature can be implemented in another command just as easily.

What alternative to virtual environments do you have in mind? I don't know any other approaches.
I just feel the need to remind you that the Python community is extremely diverse, and in trying to be inventive we can end up creating even more problems in the long run.

@golyalpha
Copy link

golyalpha commented Jul 24, 2018

Could poetry maybe output just the path to the venv on poetry env? It's what pipenv does if you give it the --venv switch.

Also, how about poetry putting it's venvs to ~/.virtualenvs, which is a path many Python IDEs search for virtual environments? Of course, an actual IDE/IDE plugin support would be ideal, but, at least in the case of VSCode's Python extension, the version not starting with 1 is an issue, apparently. See issue.

@sdispater The issue with having this info in the debug:info command is the command's name, "debug". Sounds like something you should use when you want to troubleshoot stuff, not when you want to find something out. I'm honestly for having an env command that can handle outputting this information in a bash command expansion friendly way. $(poetry env --python) . Bad example, I know, We've got poetry run for that. It's more of being able to find out where the python executable, or the entire venv, is, for the IDE's sake.

@yunti
Copy link

yunti commented Sep 5, 2018

@golyalpha Perhaps the command poetry config settings.virtualenvs.in-project true could be broadened to take an input so the venv location could be specified if needed. This would help cover a lot of scenarios and aid adoption as we wouldn't have to wait for official support for IDEs etc...

I agree with @sdispater though, it would be ideal if we generally didn't have to think about a venv. I think poetry has already done the hard parts by being able to detect them and setup one if required. The main other requirements are to view (and customise) the location and remove them.

@golyalpha definitely has a point in that debug:info isn't the most intuitive command for the venv location, but I would be hesitant to add a full env command if we are trying to abstract that away.
I'm not sure of the best place/command for it though. (show? with a --location or --where flag?)
Edit: I've since learned that the location of the venv can be shown with poetry show -v however this has the issue of creating a venv instead of just showing it, if it's missing. (as mentioned in #400 this is confusing)

As for removing a venv I would have it has a flag to remove so it can be used to remove packages, and the virtualenv too. Something like remove --all would be clear without having to think in terms of a venv.

@funkyfuture
Copy link
Contributor

beside the fact that'd i'd prefer poetry env rm, i'd find that an useful interface in automated deployment tasks.

@golyalpha
Copy link

golyalpha commented Oct 15, 2018

@yunti It's great that Poetry is trying to abstract the env away from the developer, but that is not exactly something that IDEs like. IDEs like knowing where the interpreter itself is, not how they can get to it through the usage of and external tool (in poetry's case: poetry run python), they like being able to quickly run the interpreter directly, which is also admittedly faster than going through poetry, while producing the same results.
Of course, letting the IDE add project dependencies through anything other than Poetry is a big no-no, but we already know that that isn't really an issue with VSCode and Pipenv.

TL;DR:
What I'm trying to say is, that IDEs should be able to easily access the interpreter directly, since there is no reason to prevent them from doing so and it's faster, and while it is hard (read: near impossible) to prevent an IDE from doing python -m pip install some-module, once it knows where the interpreter is, or how to get to it, I think that IDEs and their plugin developers do understand that if a project is managed by some dependency management tool, it's a good idea to not bypass it, but rather understand how it works, and use it.

Note:
Having an env command could allow project owners and it's contributors to define path to python as: $(poetry env)\Scripts\python (if bash, but it's quite similar on powershell), entirely bypassing the need for in-project venvs (where their usage is developer, not project, dependent) or having a plugin that supports Poetry when it comes to just executing the code, and providing autocomplete (of course, it's still on the dev to use poetry to install and manage deps, until IDEs/Python support plugins finally come with support for Poetry).

@sdispater
Copy link
Member

Thanks everyone for the insight and @cauebs for taking the time to make this PR!

I am closing this PR since it is superseded by #731. If you have further feedback to make please do it on #731 instead.

@sdispater sdispater closed this Dec 12, 2018
@cauebs cauebs deleted the env-command branch December 12, 2018 23:41
@cauebs
Copy link
Contributor Author

cauebs commented Dec 12, 2018

@sdispater It was more of a proof of concept. I'm quite happy with the design you ended up implementing. Great job, as usual ;)

@golyalpha golyalpha mentioned this pull request Dec 13, 2018
2 tasks
dimbleby pushed a commit to dimbleby/poetry that referenced this pull request Apr 21, 2022
* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/psf/black: 21.12b0 → 22.1.0](psf/black@21.12b0...22.1.0)

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants