Skip to content

Warn on fallback in the streaming tests in cudf-polars #19721

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

Open
wants to merge 1 commit into
base: branch-25.10
Choose a base branch
from

Conversation

Matt711
Copy link
Contributor

@Matt711 Matt711 commented Aug 18, 2025

Description

If we warn and catch it, the test will fail once we support the feature in the streaming engine. Therefore we'll keep the streaming tests up-to-date.

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 Matt711 requested a review from a team as a code owner August 18, 2025 16:28
@Matt711 Matt711 added the improvement Improvement / enhancement to an existing function label Aug 18, 2025
@Matt711 Matt711 added the non-breaking Non-breaking change label Aug 18, 2025
not maintain_order and (not is_cardinality0) and keep in {"first", "last"}
)

if should_warn:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a problem here since it's relatively simple, but if you want to avoid repeating the assert_gpu_result_equal(...) you can put just the context manager definition in the conditional:

if should_warn:
    context = pytest.warns(UserWarning, match=...)
else:
    context = contextlib.nullcontext

with context:
    assert_gpu_result_equal(q, engine=engine, check_row_order=check_row_order)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants