-
Notifications
You must be signed in to change notification settings - Fork 7k
[deps] raydepsets: building platform independent wheel #58760
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
Conversation
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
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 refactors the dependency building process to create a platform-independent wheel for raydepsets. The changes are consistent across the configuration files, build scripts, and dependency definitions. By modifying setup.py to conditionally skip binary extensions and updating the build script to generate a generic wheel, the process is simplified and no longer dependent on the Python version. The lock files have been correctly updated to reflect these changes. I have one suggestion in python/setup.py to add a comment for better code maintainability.
python/setup.py
Outdated
| cmdclass={"build_ext": build_ext}, | ||
| # The BinaryDistribution argument triggers build_ext. | ||
| distclass=BinaryDistribution, | ||
| distclass=( |
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 previous comment explaining the purpose of distclass was removed. Since the logic is now more complex, it would be beneficial for maintainability to add a new comment explaining why distclass is conditionally set to None. This will help future developers understand the intention behind this change, which is to avoid building binary extensions for 'deps-only' builds to create a platform-independent wheel.
| distclass=( | |
| distclass=( # Avoid building extensions for deps-only builds. |
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
…ub.com/ray-project/ray into elliot-barn/platform-independent-wheel
…8760) building platform independent wheel to enable users to run raydepsets on any platform Ray_img lock files unchanged only the headers have changed --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Co-authored-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
…8760) building platform independent wheel to enable users to run raydepsets on any platform Ray_img lock files unchanged only the headers have changed --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Co-authored-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com> Signed-off-by: YK <1811651+ykdojo@users.noreply.github.com>
…8760) building platform independent wheel to enable users to run raydepsets on any platform Ray_img lock files unchanged only the headers have changed --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Co-authored-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>

building platform independent wheel to enable users to run raydepsets on any platform
Ray_img lock files unchanged only the headers have changed