Skip to content

Remove ray.data code from global doctest conftest#51334

Merged
edoakes merged 17 commits intoray-project:masterfrom
edoakes:eoakes/rm-data-from-conftest
Mar 19, 2025
Merged

Remove ray.data code from global doctest conftest#51334
edoakes merged 17 commits intoray-project:masterfrom
edoakes:eoakes/rm-data-from-conftest

Conversation

@edoakes
Copy link
Copy Markdown
Collaborator

@edoakes edoakes commented Mar 13, 2025

Why are these changes needed?

Moves data-specific functionality out of the global default fixtures that are installed for doctest builds.

The way this was currently implemented basically hard-coded the global conftest.py file. I had to rework this to be installed as a pytest "plugin" using the -p argument instead. The doctest build now allows overriding the plugin file.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes requested a review from a team as a code owner March 13, 2025 13:26
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes requested a review from a team as a code owner March 13, 2025 13:32
@edoakes edoakes requested review from aslonnie and bveeramani March 13, 2025 13:32
@edoakes
Copy link
Copy Markdown
Collaborator Author

edoakes commented Mar 13, 2025

@bveeramani PTAL

@edoakes edoakes added the go add ONLY when ready to merge, run all tests label Mar 13, 2025
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes
Copy link
Copy Markdown
Collaborator Author

edoakes commented Mar 13, 2025

There is a problem with the current approach because the hard-coded conftest.py name causes a conflict.

Trying to figure out how to pass it as a "module" using pytest -p instead.

@richardliaw richardliaw added the data Ray Data-related issues label Mar 17, 2025
Copy link
Copy Markdown
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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

is this ready for review?

@edoakes
Copy link
Copy Markdown
Collaborator Author

edoakes commented Mar 18, 2025

is this ready for review?

Not yet; please hold

edoakes added 2 commits March 18, 2025 13:33
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes requested a review from aslonnie March 18, 2025 18:34
@edoakes
Copy link
Copy Markdown
Collaborator Author

edoakes commented Mar 18, 2025

@aslonnie ready now!

Comment on lines +429 to +430
"source/data/**/*.rst",
"source/data/**/*.md",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

moved to data target below

edoakes added 2 commits March 18, 2025 13:40
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes
Copy link
Copy Markdown
Collaborator Author

edoakes commented Mar 18, 2025

@bveeramani ping

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Copy link
Copy Markdown
Member

@bveeramani bveeramani left a comment

Choose a reason for hiding this comment

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

Nice, looks reasonable to me.

LGTM assuming CI runs all of the Data doctests and passes.

@edoakes
Copy link
Copy Markdown
Collaborator Author

edoakes commented Mar 18, 2025

Looks like there might be some docstrings that weren't being tested before and are being tested now... will dig

Copy link
Copy Markdown
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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


via GIPHY

edoakes added 6 commits March 19, 2025 09:29
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes enabled auto-merge (squash) March 19, 2025 19:09
@edoakes edoakes merged commit 98453a2 into ray-project:master Mar 19, 2025
5 of 6 checks passed
dhakshin32 pushed a commit to dhakshin32/ray that referenced this pull request Mar 27, 2025
Moves data-specific functionality out of the global default fixtures
that are installed for `doctest` builds.

The way this was currently implemented basically hard-coded the global
`conftest.py` file. I had to rework this to be installed as a pytest
"plugin" using the `-p` argument instead. The `doctest` build now allows
overriding the plugin file.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Dhakshin Suriakannu <d_suriakannu@apple.com>
bveeramani added a commit that referenced this pull request Aug 20, 2025
In #51334, we discovered we
weren't actually testing code snippets in our user guides. As a result,
there are several broken code snippets in our guides.

This PR fixes some of those code snippets, and re-enables testing on the
user guides.

---------

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
ljstrnadiii pushed a commit to ljstrnadiii/ray that referenced this pull request Aug 21, 2025
In ray-project#51334, we discovered we
weren't actually testing code snippets in our user guides. As a result,
there are several broken code snippets in our guides.

This PR fixes some of those code snippets, and re-enables testing on the
user guides.

---------

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
ljstrnadiii pushed a commit to ljstrnadiii/ray that referenced this pull request Aug 21, 2025
In ray-project#51334, we discovered we
weren't actually testing code snippets in our user guides. As a result,
there are several broken code snippets in our guides.

This PR fixes some of those code snippets, and re-enables testing on the
user guides.

---------

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: ljstrnadiii <ljstrnadiii@gmail.com>
jugalshah291 pushed a commit to jugalshah291/ray_fork that referenced this pull request Sep 11, 2025
In ray-project#51334, we discovered we
weren't actually testing code snippets in our user guides. As a result,
there are several broken code snippets in our guides.

This PR fixes some of those code snippets, and re-enables testing on the
user guides.

---------

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>
dstrodtman pushed a commit to dstrodtman/ray that referenced this pull request Oct 6, 2025
In ray-project#51334, we discovered we
weren't actually testing code snippets in our user guides. As a result,
there are several broken code snippets in our guides.

This PR fixes some of those code snippets, and re-enables testing on the
user guides.

---------

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Douglas Strodtman <douglas@anyscale.com>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
In ray-project#51334, we discovered we
weren't actually testing code snippets in our user guides. As a result,
there are several broken code snippets in our guides.

This PR fixes some of those code snippets, and re-enables testing on the
user guides.

---------

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-backlog data Ray Data-related issues 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