Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Clone and run Dask-cuDF tests #8

Merged
merged 8 commits into from
Feb 19, 2025
Merged

Clone and run Dask-cuDF tests #8

merged 8 commits into from
Feb 19, 2025

Conversation

rjzamora
Copy link
Member

Rough proposal/POC for running dask-cudf pytests.

Not sure if this is the "correct" approach, but I figured a rough PR would provide a reasonable space to start a discussion.

rjzamora
rjzamora commented Feb 18, 2025
View reviewed changes
TomAugspurger
TomAugspurger reviewed Feb 18, 2025
View reviewed changes
scripts/run.sh
echo "Cloning cudf@{CUDF_VERSION}"

if [ ! -d "cudf" ]; then
git clone https://github.com/rapidsai/cudf.git --branch $CUDF_VERSION
Copy link
Contributor

Choose a reason for hiding this comment

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

Something I'm trying to think through: the commit we get the nightly wheel from will potentially not match the commit we test against, whenever there are commits merged between the time the nightly wheel build runs and the time this code runs.

If there are any changes to the tests that require new code in dask_cudf, then they'll fail here.

Ideally we'd checkout the exact commit the nightly wheel was built against, but I haven't found an easy way to do that yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

If there are any changes to the tests that require new code in dask_cudf, then they'll fail here.

Aha - Great point. I don't think that's a "show stopper", but we definitely want to use the same commit if we can find a way to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, not a blocker.

@TomAugspurger
Copy link
Contributor

Overall, I think this is the right approach: clone the downstream repo and run those tests. There's a potential issue around ensuring the version installed from the nightly wheel build matches the version under test (#8 (comment)).

@rjzamora rjzamora marked this pull request as ready for review February 18, 2025 22:09
@TomAugspurger
Copy link
Contributor

TomAugspurger commented Feb 18, 2025 •
edited
Loading

FYI, CI isn't set up to run on PRs currently.

Trying this out locally, I do see a failure at dask_cudf/io/tests/test_orc.py::test_deprecated_api_paths. Do you see that too?

(I'm very confused about the failure, but haven't looked closely. It seems to be failing for the warning we expect it to throw?)

dask_cudf/io/tests/test_orc.py F                                                                                                                                                                                                 [100%]

=============================================================================================================== FAILURES ===============================================================================================================
______________________________________________________________________________________________________ test_deprecated_api_paths _______________________________________________________________________________________________________
Traceback (most recent call last):
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/runner.py", line 341, in from_call
    result: TResult | None = func()
                             ^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/runner.py", line 242, in <lambda>
    lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 182, in _multicall
    return outcome.get_result()
           ^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_result.py", line 100, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/threadexception.py", line 92, in pytest_runtest_call
    yield from thread_exception_runtest_hook()
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/threadexception.py", line 68, in thread_exception_runtest_hook
    yield
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/unraisableexception.py", line 95, in pytest_runtest_call
    yield from unraisable_exception_runtest_hook()
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/unraisableexception.py", line 70, in unraisable_exception_runtest_hook
    yield
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/logging.py", line 846, in pytest_runtest_call
    yield from self._runtest_for(item, "call")
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/logging.py", line 829, in _runtest_for
    yield
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/capture.py", line 880, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/skipping.py", line 257, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/runner.py", line 174, in pytest_runtest_call
    item.runtest()
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/python.py", line 1627, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 139, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/python.py", line 159, in pytest_pyfunc_call
    result = testfunction(**testargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nfs/toaugspurger/gh/rapidsai/dask-upstream-testing/cudf/python/dask_cudf/dask_cudf/io/tests/test_orc.py", line 150, in test_deprecated_api_paths
    with pytest.warns(match="dask_cudf.to_orc is now deprecated"):
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/raid/toaugspurger/envs/dask-upstream-testing/lib/python3.12/site-packages/_pytest/recwarn.py", line 330, in __exit__
    warnings.warn_explicit(
FutureWarning: dask_cudf.io.to_orc is now deprecated. Please use the DataFrame.to_orc method instead.

@rjzamora rjzamora changed the title [WIP] Clone and run Dask-cuDF tests Clone and run Dask-cuDF tests Feb 18, 2025
@rjzamora rjzamora marked this pull request as draft February 18, 2025 22:44
@rjzamora
Copy link
Member Author

I do see a failure at dask_cudf/io/tests/test_orc.py::test_deprecated_api_paths. Do you see that too?

I don't see that failure, but I do see a failure in test_read_orc_cols (which seems to hang) :/

@rjzamora
Copy link
Member Author

rjzamora commented Feb 19, 2025 •
edited
Loading

(I'm very confused about the failure, but haven't looked closely. It seems to be failing for the warning we expect it to throw?)

Sorry - Just looked closely at this. The failure may actually makes some sense. I think the problem is that we throw two warnings before the "correct" API is called. It's a bit strange that it doesn't fail in CI (maybe a different pytest version?).

Related PR: rapidsai/cudf#18038

@TomAugspurger TomAugspurger marked this pull request as ready for review February 19, 2025 21:57
@TomAugspurger
Copy link
Contributor

This should be good to go. Depending on whether or not rapidsai/cudf#18038 and rapidsai/cudf#18045 are merged, we might have failures in the nightly run.

I'll plan to merge this shortly, and move over to dask-cuda next.

@TomAugspurger TomAugspurger merged commit 1e82663 into rapidsai:main Feb 19, 2025
1 check passed
@rjzamora rjzamora deleted the test-dask-cudf branch February 19, 2025 22:50
@rjzamora
Copy link
Member Author

Thanks @TomAugspurger !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAugspurger TomAugspurger TomAugspurger left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rjzamora @TomAugspurger