-
Notifications
You must be signed in to change notification settings - Fork 6.9k
[doc] wheel: update steps for building and installing #58457
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
base: master
Are you sure you want to change the base?
Conversation
pip install a directory directly with setup.py is deprecated. Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
| # export GRPC_PYTHON_BUILD_SYSTEM_ZLIB=1 | ||
| pip install -e . --verbose # Add --user if you see a permission denied error. | ||
| # Build Ray wheels; install Ray and required dependencies in editable mode. | ||
| pip wheel -v -c python/requirements_compiled.txt -w .whl -e python |
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.
Bug: pip wheel: Build, Not Install
The command pip wheel -v -c python/requirements_compiled.txt -w .whl -e python only builds wheel files but doesn't install Ray. The pip wheel command creates wheel files in the .whl directory without installing them, so Ray won't be available for development. The comment claims it "install[s] Ray and required dependencies in editable mode" and the explanation states changes will "take effect without reinstalling" which is incorrect since nothing gets installed. The CI scripts show both pip install -v -e python and pip wheel -e python -w .whl are needed as separate steps.
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 is more correct..
more work to do.
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.
Code Review
This pull request updates the development setup instructions to use a more modern pip command, which is a good improvement. However, I've found a critical issue with the proposed command: it uses pip wheel with an -e flag, which is not a valid combination, and the command will fail. I've suggested a correction to use pip install instead. Additionally, the documentation text preceding the command block is now inconsistent with the new command and needs to be updated to instruct users to run it from the project root. I've left a comment with a suggestion for this as well.
| # export GRPC_PYTHON_BUILD_SYSTEM_ZLIB=1 | ||
| pip install -e . --verbose # Add --user if you see a permission denied error. | ||
| # Build Ray wheels; install Ray and required dependencies in editable mode. | ||
| pip wheel -v -c python/requirements_compiled.txt -w .whl -e python |
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.
The command as written is invalid because pip wheel does not support the -e (--editable) flag. This appears to mix options from pip wheel (-w) and pip install (-e) and will cause the command to fail.
To install Ray in editable mode, you should use pip install. If you also need to build wheels into a directory, that would be a separate pip wheel command. Assuming the primary goal is the editable install, here's a corrected command:
| pip wheel -v -c python/requirements_compiled.txt -w .whl -e python | |
| pip install -v -c python/requirements_compiled.txt -e python |
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.
https://pip.pypa.io/en/stable/cli/pip_wheel/#cmdoption-e
wake up, gemini!
| # Build Ray wheels; install Ray and required dependencies in editable mode. | ||
| pip wheel -v -c python/requirements_compiled.txt -w .whl -e python |
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.
The instruction in the line preceding this code block, Enter into the ``python/`` directory..., is now inconsistent with the new command. The paths used here (e.g., python/requirements_compiled.txt and -e python) indicate that the command should be run from the project's root directory, not from within the python/ directory.
To prevent user confusion, please update the instructional text. For example:
From the Ray project root directory, run the following command to build and install Ray:
pip install a directory directly with setup.py is deprecated.