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 --python-path, --pip-path, --gradio-path CLI arguments to let custom component developers control which executable is used #7638

Merged
merged 9 commits into from Mar 8, 2024

Conversation

freddyaboulton
Copy link
Collaborator

Description

Closes: #7621

Adds CLI arguments to let developers control the path of the python executables used in create, dev, and build commands.

Also adds some debugging information to dev mode in case no custom components are found. I suspect this will help debug issues like #6821

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Tests

  1. PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh

  2. You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Mar 7, 2024

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
Storybook failed! Details
🦄 Changes detecting...

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/595361c1c31ce4e9725c25c31baaed0040233f6b/gradio-4.20.1-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@595361c1c31ce4e9725c25c31baaed0040233f6b#subdirectory=client/python"

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Mar 7, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/preview patch
gradio patch
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Add --python-path, --pip-path, --gradio-path CLI arguments to let custom component developers control which executable is used

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@freddyaboulton freddyaboulton marked this pull request as ready for review March 7, 2024 20:21
@abidlabs
Copy link
Member

abidlabs commented Mar 7, 2024

Nice @freddyaboulton! Just to understand, when would one use --gradio-path?

@freddyaboulton
Copy link
Collaborator Author

We used to use which to determine the path of the gradio executable so we could launch the demo in reload mode, e.g. gradio demo/app.py. But it's possible that it picks up the wrong version of gradio if multiple versions of gradio are installed in the same system. So I'm adding a CLI argument to let developers take control if something goes wrong.

@abidlabs
Copy link
Member

abidlabs commented Mar 8, 2024

Thanks @freddyaboulton for this PR! While I think this is a working solution, it seems like a bit of a headache for the user to have to find and pass in the path to their Python for the CLI commands, particularly as they need to do it for each gradio cc command. We can add these arguments, but could we also handle the default case better:

What if we resolved pip in this order:

  • the pip_path cli arguments, if provided, otherwise:
  • which("pip3") if exists, otherwise
  • which("pip") if exists, otherwise
  • fail loudly

Same for python3 and python. I think that should make the default behavior much better for the majority of users

@freddyaboulton
Copy link
Collaborator Author

it seems like a bit of a headache for the user to have to find and pass in the path to their Python for the CLI commands, particularly as they need to do it for each gradio cc command.

I don't follow. Passing in the -path parameters is not required and is only needed if whatever heuristic we have in place fails. We can also check for python3/pip3 though.

@abidlabs
Copy link
Member

abidlabs commented Mar 8, 2024

I don't follow. Passing in the -path parameters is not required and is only needed if whatever heuristic we have in place fails. We can also check for python3/pip3 though.

Sorry my earlier comment may not have been clear. I agree -- let's keep these parameters but check for python3 and pip3 as well

@freddyaboulton
Copy link
Collaborator Author

Sounds good @abidlabs ! Just pushed up that change!

@abidlabs
Copy link
Member

abidlabs commented Mar 8, 2024

Code changes look good! In my tests, --pip-path is working, but --python-path is not having any effect.

For example, I can run:

gradio cc dev --python-path abdef/asdfklasdjf

after creating a custom component and the dev server launches successfully. That's not expected, right?

@freddyaboulton
Copy link
Collaborator Author

freddyaboulton commented Mar 8, 2024

@abidlabs I think you needed to built the js for this PR for dev mode to break in that case. But your comment made me realize we wouldn't raise a helpful error if the executable did not exist so I just pushed that up.

@abidlabs
Copy link
Member

abidlabs commented Mar 8, 2024

Ah I see, thanks I'll give this another spin

Copy link
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful. LGTM

@abidlabs
Copy link
Member

abidlabs commented Mar 8, 2024

Going to merge this in for the release @freddyaboulton

@abidlabs abidlabs merged commit b3b0ea3 into main Mar 8, 2024
6 of 7 checks passed
@abidlabs abidlabs deleted the 7621-cc-executable-path branch March 8, 2024 23:01
@pngwn pngwn mentioned this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gradio cc create Command Fails Without Virtual Environment Due to pip Path Issue
3 participants