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.

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

[BUG] Investigate Narwhals tests failures when run with cudf.pandas #18248

Open
Matt711 opened this issue Mar 12, 2025 · 5 comments
Open

[BUG] Investigate Narwhals tests failures when run with cudf.pandas #18248

Matt711 opened this issue Mar 12, 2025 · 5 comments
Assignees
Labels
bug Something isn't working Needs Triage Need team to review and classify

Comments

@Matt711
Copy link
Contributor

Matt711 commented Mar 12, 2025

Describe the bug
In #18031, we started running the Narwhals test suite with cudf.pandas. There are several failing tests that we needed to skip (listed in https://github.com/rapidsai/cudf/blob/branch-25.04/ci/test_narwhals.sh). We should either fix these failures or explain why they won't be fixed.

Steps/Code to reproduce bug
Follow the steps in this shell scripts (eg. clone Narwhals, install dependencies, etc.).

Additional context
Please xref this issue once you've fixed a test

Original Issue to add Narwhals tests to cuDF: #17662

@Matt711 Matt711 added the bug Something isn't working label Mar 12, 2025
@Matt711 Matt711 added the Needs Triage Need team to review and classify label Mar 12, 2025
rapids-bot bot pushed a commit that referenced this issue Mar 12, 2025
Skips the tests for now so that nightly CI passes. We need to investigate the tests we skip to see which ones can be fixed. Tracking Issue for that work is in #18248

Authors:
  - Matthew Murray (https://github.com/Matt711)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #18249
@galipremsagar
Copy link
Contributor

I'm investigating the cross join test failures.

@mroeschke
Copy link
Contributor

#18255 will help partially resolve the skipped test_pandas_object_series test, but I also think Narwhals would need to add an explicit dtype=object call here in https://github.com/narwhals-dev/narwhals/blob/af20c95c1e35e3dd4183b7d001ea69fd142d718b/tests/expr_and_series/list/len_test.py#L66

since cudf infers that data as ListDtype and pandas infers that data as object by default

@mroeschke
Copy link
Contributor

test_array_dunder_with_copy[pandas] I think should always be skipped. It's testing numpy copy behavior with __array__, and we always make a copy to CPU here

@mroeschke
Copy link
Contributor

tests/utils_test.py::test_maybe_convert_dtypes_pandas should fail until #14149 is addressed

@mroeschke
Copy link
Contributor

tests/series_only/to_arrow_test.py::test_to_arrow_with_nulls[pandas] and tests/frame/to_arrow_test.py::test_to_arrow[pandas] look OK to skip.

They are testing a Narwhals implementation of to_arrow which looks to have different behavior whether cudf is involved or not https://github.com/narwhals-dev/narwhals/blob/af20c95c1e35e3dd4183b7d001ea69fd142d718b/narwhals/_pandas_like/series.py#L879. This test doesn't really consider if cudf.pandas is loaded (nor should really care)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Needs Triage Need team to review and classify
Projects
None yet
Development

No branches or pull requests

3 participants