-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Update pip install calls in scripts to use uv or reference current executable #13597
base: main
Are you sure you want to change the base?
Conversation
CONTRIBUTING.md
Outdated
replacing `$INSERT_LIBRARY_NAME_HERE` with the name of the library: | ||
|
||
```bash | ||
(.venv)$ pip install $INSERT_LIBRARY_NAME_HERE | ||
(.venv)$ uv pip install $INSERT_LIBRARY_NAME_HERE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think contributors shouldn't need to know about uv
. While I'm fine with using uv
to speed up our CI, it's not something I would personally use when developing locally, and I don't think it solves a problem for the typical contributor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do use uv
all the time locally, but I'd also much prefer it if new contributors don't have to worry about it. I really want new contributors to be able to get up and running as quickly as possible on typeshed; the setup steps should be as simple as possible and involve the bare minimum of custom tools IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only changed places where I'd assume pip install -r requirements-tests.txt
was already run.
Using it in doc was less about install speed, and more about not having to worry about virtual environments, and whether it exists / is activated.
I could see being told to run a custom tool (uv) when you may not even know that it was part of the installed requirements to be jarring.
I've updated the PR with that in mind.
…l_third_party_dependencies`
|
||
def main() -> None: | ||
requirements = get_external_stub_requirements() | ||
subprocess.check_call(("uv", "pip", "install", *sys.argv[1:], *[str(requirement) for requirement in requirements])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This forces working with a virtual environment for those scripts, but it doesn't force a name on your venv (
uv pip
anduv run --active
will reuse your activated environment)Note I didn't touch
python
/python3
-->uv run
in this PR. (that's done in #13599)I'm also leaving GitHub jobs to a separate self-contained PR.
I changed lists to tuples on relevant lines.