-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
Detect .venv and add maturin run
#1461
Comments
1462: Detect .venv in current or parent folder in maturin develop r=messense a=konstin See discussion in #1461 --- `maturin develop` now looks for a virtualenv .venv in the current or any parent directory if no venv or conda environment is active. --- If someone knows a good `execv` equivalent on windows i'll switch over to that, currently it's a subcommand which is still fine. Co-authored-by: konstin <[email protected]> Co-authored-by: messense <[email protected]>
Would you also be willing to support discovery of |
Could you summarize what the motivation for having Logic itself isn't much overhead, but i'm worried about how this is will confuse inexperienced users when something with their environment goes wrong and that this would make all error reporting and configuration more complex and documentation way more verbose (e.g. instead of "look inside the .venv" it becomes "if .venv is a directory, look inside, if it is a file, look inside the path pointed to by the text inside the .venv file", users most likely want additional options to create the venv else for automatic venv creation, etc.), so for me this would need a strong use case to be included. |
One thing I like about it is that it makes it really easy to switch between venv's that have different Python versions. |
couldn't that be done faster by activating another venv (which has precedence over .venv) instead of editing the file? |
The idea is that this would provide a way for all tools — including things that aren't CLIs, like my IDE — to know what environment I want to use. (Also in practice I'm not editing the file by hand, I have some tool that is) |
I don't think the discuss thread was concluded, although the poll looked positive: https://discuss.python.org/t/setting-up-some-guidelines-around-discovering-finding-naming-virtual-environments/22922/32 so if you're not sure this would be good for your users, that's okay! :-) |
I’m not too familiar with how Python virtualenvs work these days but I world try to mirror how pip installed Python tools work. They basically get the shebang updated to the Python interpreter of the venv injected. I think the equivalent would be to try to discover an adjacent Python interpreter from the maturin executable and the activation script? That assumes maturin is installed in the virtualenv of course. |
Maybe I'm misunderstanding what maturin does, but this idea sounds a bit like feature creep to me... I don't know any other Python build tool or installer that actually tries to detect and activate a virtualenv? I know that setting The only ones that come to mind are poetry and virtualenv itself, and they are the ones that are actually creating and managing virtualenvs, and not just installing, building, or running something in it. I think I would like to see maturin encourage the use of a virtualenv, but I don't want it to try and manage it for me. Same with |
From my perspective, everything that is required to make maturin just work is in scope. You should be able to do
The new generation of package managers such a poetry, pdm and hatch all include similar venv management. rye additionally installs shims that make Ask differently, what problem does it cause if maturin uses an existing the venv where it would otherwise fail? |
But in order to install maturin I've already created a venv and chosen a package installer, no? Do I then have to switch gears and stop using those tools? Doesn't that make installing maturin harder and not easier?
You mean Python requirements? Why? There are already numerous tools that try to do that. Adding yet another option will make things harder, not easier.... Reading #284 it seems like you heard the criticism, but then went in the opposite direction... To me it reads like people want maturin to
Yet we ended up with maturin now wanting to create and manage my virtualenv?
You'll be limiting yourself to compatible venv structures only. Not having a venv is a structure that is very common in containers! Plus people will get confused, because that's not how Python tools usually operate. Usually they do the two things mentioned above. |
Not necessarily, many people have a global install or use something like
We're not adding a standard, on the contrary, there used to be various more-or-less specified format such as as
I would like to do this but this fails when installed through pipx because pipx will install maturin in a venv, but the user definitely doesn't want to have their development package installed into the pipx environment. maturin instead checks for the users active venv first.
Yeah this is an inconvenience in containers, but only in the edge case where you want to |
You're right, but requiring users to break with their tried and trusted tools is bad UX too. Python already has virtualenv and package managers, why force them to break with it? That's like asking people to abandon
Why do you want to install
Sure, a dependency format, but I'm talking about package installers. Reading a dependency list is easy, solving and installing them is the hard part.
To me that's not an edgecase but basically every CI pipeline out there. |
I don’t like the idea. |
Currently,
maturin develop
will pick up an activated venv or conda and will otherwise fail. Inspired by PEP 704 and the discussion around it, i've implemented a new behaviour where it also picks up a virtualenv called.venv
in the current directory or any parent directory, i.e. you don't need to manually activate the venv anymore. You can then usematurin run
to actually run your python code, e.g.maturin run -m pytest
.I've implemented this in #1462.
CC @mitsuhiko would this solve #284 (comment)?
The text was updated successfully, but these errors were encountered: