Enable third party library integration tests in CI with `cudf.pandas` #17936

galipremsagar · 2025-02-06T20:22:28Z

Description

This PR enables 3rd party library integration tests that are run with cudf.pandas enabled.

Fixes: rapidsai/cuml#6301

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

Matt711

Does this ensure that the tests are run with with the changes to cudf in the current PR? I'm not sure it does.

vyasr · 2025-02-06T20:36:22Z

Good call Matt. We will probably have to update https://github.com/rapidsai/cudf/blob/branch-25.04/ci/cudf_pandas_scripts/third-party-integration/test.sh to support downloading CI artifacts like in other jobs.

galipremsagar · 2025-02-06T22:47:43Z

/okay to test

…arty

Matt711

Thanks Prem! This looks good to me once CI passes

Matt711 · 2025-02-07T16:00:07Z

python/cudf/cudf/pandas/_wrappers/pandas.py

+def wrap_from_pandas(original_call):
+    @functools.wraps(original_call)
+    def wrapped_from_pandas(obj, *args, **kwargs):
+        if is_proxy_object(obj):
+            obj = obj.as_gpu_object()
+            return obj
+        return original_call(obj, *args, **kwargs)
+
+    return wrapped_from_pandas
+
+
+def wrap_from_pandas_dataframe(original_call):
+    @functools.wraps(original_call)
+    def wrapped_from_pandas_dataframe(dataframe, *args, **kwargs):
+        if is_proxy_object(dataframe):
+            dataframe = dataframe.as_gpu_object()
+            if isinstance(dataframe, cudf.DataFrame):
+                return dataframe
+        return original_call(dataframe, *args, **kwargs)
+
+    return wrapped_from_pandas_dataframe
+
+
+def wrap_from_pandas_series(original_call):
+    @functools.wraps(original_call)
+    def wrapped_from_pandas_series(s, *args, **kwargs):
+        if is_proxy_object(s):
+            s = s.as_gpu_object()
+            if isinstance(s, cudf.Series):
+                return s
+        return original_call(s, *args, **kwargs)
+
+    return wrapped_from_pandas_series
+
+
+def wrap_from_pandas_index(original_call):
+    @functools.wraps(original_call)
+    def wrapped_from_pandas_index(index, *args, **kwargs):
+        if is_proxy_object(index):
+            index = index.as_gpu_object()
+            if isinstance(index, cudf.core.index.BaseIndex):
+                return index
+        return original_call(index, *args, **kwargs)
+
+    return wrapped_from_pandas_index
+
+
+def wrap_from_pandas_multiindex(original_call):
+    @functools.wraps(original_call)
+    def wrapped_from_pandas_multiindex(multiindex, *args, **kwargs):
+        if is_proxy_object(multiindex):
+            multiindex = multiindex.as_gpu_object()
+            if isinstance(multiindex, cudf.MultiIndex):
+                return multiindex
+        return original_call(multiindex, *args, **kwargs)
+
+    return wrapped_from_pandas_multiindex


Could this work?

def wrap_from_pandas(original_call, type_=None): @functools.wraps(original_call) def wrapped(obj, *args, **kwargs) if is_proxy_object(obj): obj = obj.as_gpu_object() if (type_ is None) or isinstance(obj, type_): return obj return original_call(obj, *args, **kwargs) return wrapped

The problem will arise if some passed keywords like cudf.DataFrame.from_pandas(dataframe=df) , cudf.Series.from_pandas(s=ser). Your suggestion was the way I initially wanted to go by but all the functions have different keyword argument names so I had to separate out the implementations.

We could presumably handle that in kwargs in the unified wrapper? This isn't a blocker btw

That approach might get very complicated with a bunch of if/elif's to handle all possible combinations of each constructor.

Matt711 · 2025-02-07T16:00:36Z

python/cudf/cudf/pandas/_wrappers/pandas.py

+    cudf.from_pandas = wrap_from_pandas(_original_from_pandas)
+    cudf.DataFrame.from_pandas = wrap_from_pandas_dataframe(
+        _original_DataFrame_from_pandas
+    )
+    cudf.Series.from_pandas = wrap_from_pandas_series(
+        _original_Series_from_pandas
+    )
+    cudf.BaseIndex.from_pandas = wrap_from_pandas_index(
+        _original_Index_from_pandas
+    )
+    cudf.MultiIndex.from_pandas = wrap_from_pandas_multiindex(
+        _original_MultiIndex_from_pandas
+    )


And then use functools.partial

Suggested change

cudf.from_pandas = wrap_from_pandas(_original_from_pandas)

cudf.DataFrame.from_pandas = wrap_from_pandas_dataframe(

_original_DataFrame_from_pandas

)

cudf.Series.from_pandas = wrap_from_pandas_series(

_original_Series_from_pandas

)

cudf.BaseIndex.from_pandas = wrap_from_pandas_index(

_original_Index_from_pandas

)

cudf.MultiIndex.from_pandas = wrap_from_pandas_multiindex(

_original_MultiIndex_from_pandas

)

cudf.from_pandas = partial(wrap_from_pandas, type=None)

cudf.DataFrame.from_pandas = partial(wrap_from_pandas, type_=cudf.DataFrame)

...

Same as: #17936 (comment)

mroeschke · 2025-02-07T19:43:23Z

python/cudf/cudf/pandas/_wrappers/pandas.py

+def wrap_from_pandas_dataframe(original_call):
+    @functools.wraps(original_call)
+    def wrapped_from_pandas_dataframe(dataframe, *args, **kwargs):
+        if is_proxy_object(dataframe):


For wrapping these classmethods, wouldn't dataframe be cls here? We want to be extracting cudf.pandas objects which might be in args or kwargs?

It would be nice to match the method signatures exactly as well if possible

I agree, should we plan on deprecating the arguments to name them in a unified fashion? or will that be too much noise for the warnings and rather should it just be a breaking change?

I actually just meant making the wrapped function signature here match the <obj>.from_pandas signature e.g.

# nan_as_null=False matches what would happen when evaluating the mode.pandas_compatible setting in cudf.pandas def wrapped_from_pandas_dataframe(cls, dataframe, nan_as_null=False): ...

As for the API change, I suppose we could do that as a breaking change, but I don't think we absolutely need to align the signatures of from_pandas

galipremsagar · 2025-02-09T13:53:37Z

Hey @galipremsagar can we just add the CI changes in this PR and skip any of the failing tests? We can open targeted PRs to fix the remaining tests afterwards.

Yes, on it.

ci/cudf_pandas_scripts/third-party-integration/test.sh

ci/cudf_pandas_scripts/third-party-integration/run-library-tests.sh

galipremsagar · 2025-02-10T00:14:06Z

@Matt711 The pr is now ready for review. Can you sign off on it?

Matt711

Sorry I have a blocking question about the stumpy failures because they seem new to me.

Matt711 · 2025-02-10T00:46:17Z

python/cudf/cudf_pandas_tests/third_party_integration_tests/tests/test_stumpy_distributed.py

@@ -30,13 +30,15 @@ def dask_client():
            yield dask_client


+@pytest.mark.skip(reason="TODO: Fix these stumpy tests to work with dask")


I haven't seen these stumpy failures before. Is the monkey patching in this PR causing these failures?

Do you mind checking if you haven't? It it is, I'd rather xfail the cugraph tests in this PR (which I think are passing now). And follow-up and add the monkey patching in a way that the integration tests pass (cugraph and stumpy).

I could be wrong about the monkeypatching causing the stumpy failures.

It is not being caused by monkey-patching from_pandas in this PR. The stumpy distributed failures seem to be related to some ptx failures. I'm investigating it, but I didn't want them to block this PR. Let's proceed with this PR and then tackle the stumpy failures separately?

Yup, thanks for looking into it!

Matt711 · 2025-02-10T00:46:39Z

python/cudf/cudf_pandas_tests/third_party_integration_tests/dependencies.yaml

@@ -262,7 +262,7 @@ dependencies:
        packages:
          - pip
          - pip:
-              - ibis-framework[pandas]
+              - ibis-framework[pandas]<10.0.0


This is fine for now, I'll remove it in #17945

Yes, that was my intention behind this change.

Matt711

Thanks @galipremsagar! Matt R had a comment about making the classmethods match the signatures of the methods they're patching. But I think that can be added in a follow up.

galipremsagar · 2025-02-10T13:39:55Z

Thanks @galipremsagar! Matt R had a comment about making the classmethods match the signatures of the methods they're patching. But I think that can be added in a follow up.

Responded here: #17936 (comment)

galipremsagar · 2025-02-10T13:40:01Z

/merge

enable 3rd party tests

82770fd

galipremsagar added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 6, 2025

galipremsagar self-assigned this Feb 6, 2025

galipremsagar requested a review from a team as a code owner February 6, 2025 20:22

galipremsagar requested a review from msarahan February 6, 2025 20:22

Merge branch 'branch-25.04' into enable_3rdparty

e371dc9

msarahan approved these changes Feb 6, 2025

View reviewed changes

Matt711 requested changes Feb 6, 2025

View reviewed changes

galipremsagar added 6 commits February 6, 2025 13:11

test

64e3aeb

test

5404d7e

test

3d308ff

update script

6a5c213

update

18d195c

fix the condition

58d6066

This was referenced Feb 6, 2025

Make cudf.pandas proxy array picklable #17929

Merged

Remove pandas backend from cudf.pandas - ibis integration tests #17945

Merged

galipremsagar added 3 commits February 6, 2025 20:28

Merge branch 'branch-25.04' into enable_3rdparty

2501ce3

Merge remote-tracking branch 'upstream/branch-25.04' into enable_3rdp…

fd14543

…arty

add from_pandas proxies

00e41be

galipremsagar requested a review from a team as a code owner February 7, 2025 15:21

galipremsagar requested review from vyasr and Matt711 February 7, 2025 15:21

github-actions bot added Python Affects Python cuDF API. cudf.pandas Issues specific to cudf.pandas labels Feb 7, 2025

Matt711 reviewed Feb 7, 2025

View reviewed changes

fix

83c6c8b

mroeschke reviewed Feb 7, 2025

View reviewed changes

galipremsagar added 3 commits February 9, 2025 08:02

Update test.sh

6729b83

Merge branch 'branch-25.04' into enable_3rdparty

ebbca84

Update run-library-tests.sh

9e1d63c

galipremsagar commented Feb 9, 2025

View reviewed changes

ci/cudf_pandas_scripts/third-party-integration/test.sh Outdated Show resolved Hide resolved

galipremsagar added 2 commits February 9, 2025 08:15

Update ci/cudf_pandas_scripts/third-party-integration/test.sh

717ac83

Update test_stumpy_distributed.py

4062b9b

galipremsagar commented Feb 9, 2025

View reviewed changes

ci/cudf_pandas_scripts/third-party-integration/run-library-tests.sh Outdated Show resolved Hide resolved

galipremsagar commented Feb 9, 2025

View reviewed changes

ci/cudf_pandas_scripts/third-party-integration/run-library-tests.sh Outdated Show resolved Hide resolved

galipremsagar added 6 commits February 9, 2025 11:06

Apply suggestions from code review

149cc09

Update test_stumpy_distributed.py

895fd17

rm

d063708

Update test_stumpy_distributed.py

747b159

skip

1fb3831

pin ibis

eda3cdf

galipremsagar requested a review from a team as a code owner February 9, 2025 22:51

galipremsagar requested a review from Matt711 February 10, 2025 00:19

Matt711 reviewed Feb 10, 2025

View reviewed changes

Matt711 approved these changes Feb 10, 2025

View reviewed changes

galipremsagar mentioned this pull request Feb 10, 2025

[BUG] test_predict_non_gaussian failing: "Could not construct DataFrame from <class 'pandas.core.frame.DataFrame'>" rapidsai/cuml#6301

Closed

galipremsagar added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Feb 10, 2025

rapids-bot bot merged commit 218d67d into rapidsai:branch-25.04 Feb 10, 2025
122 checks passed

galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Feb 10, 2025

galipremsagar mentioned this pull request Feb 10, 2025

[BUG] cuGraph integration tests fail with TypeError: Could not construct DataFrame from <class 'pandas.core.frame.DataFrame'> #17934

Closed

Matt711 mentioned this pull request Feb 10, 2025

Add catboost integration tests #17931

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable third party library integration tests in CI with `cudf.pandas` #17936

Enable third party library integration tests in CI with `cudf.pandas` #17936

galipremsagar commented Feb 6, 2025 •

edited

Loading

Matt711 left a comment

vyasr commented Feb 6, 2025 •

edited

Loading

galipremsagar commented Feb 6, 2025

Matt711 left a comment

Matt711 Feb 7, 2025

galipremsagar Feb 7, 2025

Matt711 Feb 7, 2025

galipremsagar Feb 10, 2025

Matt711 Feb 7, 2025

galipremsagar Feb 7, 2025

mroeschke Feb 7, 2025 •

edited

Loading

galipremsagar Feb 10, 2025 •

edited

Loading

mroeschke Feb 10, 2025

galipremsagar commented Feb 9, 2025

galipremsagar commented Feb 10, 2025

Matt711 left a comment

Matt711 Feb 10, 2025

Matt711 Feb 10, 2025 •

edited

Loading

Matt711 Feb 10, 2025

galipremsagar Feb 10, 2025

Matt711 Feb 10, 2025

Matt711 Feb 10, 2025

galipremsagar Feb 10, 2025

Matt711 left a comment

galipremsagar commented Feb 10, 2025

galipremsagar commented Feb 10, 2025

		@@ -30,13 +30,15 @@ def dask_client():
		yield dask_client


		@pytest.mark.skip(reason="TODO: Fix these stumpy tests to work with dask")

Enable third party library integration tests in CI with cudf.pandas #17936

Enable third party library integration tests in CI with cudf.pandas #17936

Conversation

galipremsagar commented Feb 6, 2025 • edited Loading

Description

Checklist

Matt711 left a comment

Choose a reason for hiding this comment

vyasr commented Feb 6, 2025 • edited Loading

galipremsagar commented Feb 6, 2025

Matt711 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

galipremsagar Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galipremsagar commented Feb 9, 2025

galipremsagar commented Feb 10, 2025

Matt711 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Matt711 Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Matt711 left a comment

Choose a reason for hiding this comment

galipremsagar commented Feb 10, 2025

galipremsagar commented Feb 10, 2025

Enable third party library integration tests in CI with `cudf.pandas` #17936

Enable third party library integration tests in CI with `cudf.pandas` #17936

galipremsagar commented Feb 6, 2025 •

edited

Loading

vyasr commented Feb 6, 2025 •

edited

Loading

mroeschke Feb 7, 2025 •

edited

Loading

galipremsagar Feb 10, 2025 •

edited

Loading

Matt711 Feb 10, 2025 •

edited

Loading