Skip to content

[Docs] Fix incorrect code output in writing-code-snippets doc#60351

Merged
aslonnie merged 1 commit intomasterfrom
gs
Jan 21, 2026
Merged

[Docs] Fix incorrect code output in writing-code-snippets doc#60351
aslonnie merged 1 commit intomasterfrom
gs

Conversation

@bveeramani
Copy link
Copy Markdown
Member

#59631 changed the way the Dataset representations look, but CI didn't test writing-code-snippet in that PR's premerge CI. This PR fixes the incorrect output.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
@bveeramani bveeramani requested a review from a team as a code owner January 21, 2026 02:12
Copy link
Copy Markdown
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 correctly updates a code example in the writing-code-snippets.rst documentation to match the new ray.data.Dataset string representation. The changes, including updating the .. testoutput:: block and using np.round for cleaner output, are accurate and improve the documentation. I have one minor suggestion to improve the readability of the example code.

Comment on lines 156 to +158
vec_a = batch["petal length (cm)"]
vec_b = batch["petal width (cm)"]
batch["petal area (cm^2)"] = vec_a * vec_b
batch["petal area (cm^2)"] = np.round(vec_a * vec_b, 2)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For better readability and to set a good example in this guide on writing code snippets, consider using more descriptive variable names instead of vec_a and vec_b. For example, petal_length and petal_width would make the code's intent clearer.

Suggested change
vec_a = batch["petal length (cm)"]
vec_b = batch["petal width (cm)"]
batch["petal area (cm^2)"] = vec_a * vec_b
batch["petal area (cm^2)"] = np.round(vec_a * vec_b, 2)
petal_length = batch["petal length (cm)"]
petal_width = batch["petal width (cm)"]
batch["petal area (cm^2)"] = np.round(petal_length * petal_width, 2)

│ --- ┆ --- ┆ --- ┆ --- ┆ --- ┆ --- │
│ double ┆ double ┆ double ┆ double ┆ int64 ┆ double │
╞═══════════════════╪══════════════════╪═══════════════════╪══════════════════╪════════╪═══════════════════╡
│ 5.1 ┆ 3.5 ┆ 1.4 ┆ 0.2 ┆ 0 ┆ 0.28 │
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

these are some very specific data.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's the data in the canonical Iris dataset. We should enable preserve_order for Data doctest, so these results should be deterministic

@aslonnie aslonnie enabled auto-merge (squash) January 21, 2026 02:15
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Jan 21, 2026
@aslonnie aslonnie self-requested a review January 21, 2026 02:15
Copy link
Copy Markdown
Contributor

@richardliaw richardliaw left a comment

Choose a reason for hiding this comment

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

any reason how this wasn't caught in earlier tests?

@bveeramani
Copy link
Copy Markdown
Member Author

@richardliaw I guess the contributor guide doctests don't run on Data premerge

@aslonnie aslonnie disabled auto-merge January 21, 2026 02:42
@aslonnie aslonnie merged commit b48817c into master Jan 21, 2026
5 of 7 checks passed
@aslonnie aslonnie deleted the gs branch January 21, 2026 02:42
abrarsheikh pushed a commit that referenced this pull request Jan 21, 2026
#59631 changed the way the
`Dataset` representations look, but CI didn't test
`writing-code-snippet` in that PR's premerge CI. This PR fixes the
incorrect output.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: abrar <abrar@anyscale.com>
jinbum-kim pushed a commit to jinbum-kim/ray that referenced this pull request Jan 29, 2026
…project#60351)

ray-project#59631 changed the way the
`Dataset` representations look, but CI didn't test
`writing-code-snippet` in that PR's premerge CI. This PR fixes the
incorrect output.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: jinbum-kim <jinbum9958@gmail.com>
400Ping pushed a commit to 400Ping/ray that referenced this pull request Feb 1, 2026
…project#60351)

ray-project#59631 changed the way the
`Dataset` representations look, but CI didn't test
`writing-code-snippet` in that PR's premerge CI. This PR fixes the
incorrect output.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: 400Ping <jiekaichang@apache.org>
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Feb 3, 2026
…project#60351)

ray-project#59631 changed the way the
`Dataset` representations look, but CI didn't test
`writing-code-snippet` in that PR's premerge CI. This PR fixes the
incorrect output.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
peterxcli pushed a commit to peterxcli/ray that referenced this pull request Feb 25, 2026
…project#60351)

ray-project#59631 changed the way the
`Dataset` representations look, but CI didn't test
`writing-code-snippet` in that PR's premerge CI. This PR fixes the
incorrect output.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: peterxcli <peterxcli@gmail.com>
peterxcli pushed a commit to peterxcli/ray that referenced this pull request Feb 25, 2026
…project#60351)

ray-project#59631 changed the way the
`Dataset` representations look, but CI didn't test
`writing-code-snippet` in that PR's premerge CI. This PR fixes the
incorrect output.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: peterxcli <peterxcli@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.

3 participants