Skip to content

Conversation

@eicherseiji
Copy link
Contributor

@eicherseiji eicherseiji commented Oct 24, 2025

Description

  • Markdown instructions out of date with notebook contents
  • Notebooks are already tested in CI, but with ray-llm image
  • ray[llm] already bundles data, and is used for ray-llm image, so it more accurately reflects the tested configuration

Additional information

Manual tests complete successfully with rayproject/ray-llm:2.50.1-py311-cu128:
Screenshot 2025-10-24 at 2 59 22 PM
Screenshot 2025-10-24 at 2 54 16 PM

Signed-off-by: Seiji Eicher <seiji@anyscale.com>
Signed-off-by: Seiji Eicher <seiji@anyscale.com>
@eicherseiji eicherseiji requested a review from a team as a code owner October 24, 2025 20:00
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 installation instructions in two example notebooks to use ray[llm], which is a good step for consistency. The change for vllm-with-lora.ipynb looks correct. However, for vllm-with-structural-output.ipynb, the xgrammar dependency was removed. Since ray[llm] does not appear to install xgrammar, this could break the notebook for users. I've added a comment with a suggestion to re-introduce xgrammar to the installation command for that specific notebook.

@eicherseiji eicherseiji added the go add ONLY when ready to merge, run all tests label Oct 24, 2025
@kouroshHakha kouroshHakha merged commit 05c0aff into ray-project:master Oct 24, 2025
6 checks passed
xinyuangui2 pushed a commit to xinyuangui2/ray that referenced this pull request Oct 27, 2025
Signed-off-by: Seiji Eicher <seiji@anyscale.com>
Signed-off-by: xgui <xgui@anyscale.com>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
Signed-off-by: Seiji Eicher <seiji@anyscale.com>
Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request Nov 19, 2025
Signed-off-by: Seiji Eicher <seiji@anyscale.com>
Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request Dec 7, 2025
Signed-off-by: Seiji Eicher <seiji@anyscale.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants