-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Clean up legacy test scaffolding in cudf-polars #22535
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
Merged
rapids-bot
merged 3 commits into
rapidsai:release/26.06
from
madsbk:cleanup_legacy_tests
May 18, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,6 @@ def assert_gpu_result_equal( | |
| engine: GPUEngine, | ||
| collect_kwargs: CollectKwargs | None = None, | ||
| polars_collect_kwargs: CollectKwargs | None = None, | ||
| cudf_collect_kwargs: CollectKwargs | None = None, | ||
| check_row_order: bool = True, | ||
| check_column_order: bool = True, | ||
| check_dtypes: bool = True, | ||
|
|
@@ -59,10 +58,6 @@ def assert_gpu_result_equal( | |
| Keyword arguments to pass to collect for execution on polars CPU. | ||
| Overrides kwargs in collect_kwargs. | ||
| Useful for controlling optimization settings. | ||
| cudf_collect_kwargs | ||
| Keyword arguments to pass to collect for execution on cudf-polars. | ||
| Overrides kwargs in collect_kwargs. | ||
| Useful for controlling optimization settings. | ||
| check_row_order | ||
| Expect rows to be in same order | ||
| check_column_order | ||
|
|
@@ -86,14 +81,13 @@ def assert_gpu_result_equal( | |
| NotImplementedError | ||
| If GPU collection failed in some way. | ||
| """ | ||
| final_polars_collect_kwargs, final_cudf_collect_kwargs = _process_kwargs( | ||
| collect_kwargs, polars_collect_kwargs, cudf_collect_kwargs | ||
| ) | ||
| gpu_kwargs = collect_kwargs or {} | ||
| cpu_kwargs = gpu_kwargs | (polars_collect_kwargs or {}) | ||
|
|
||
| # These keywords are correct, but mypy doesn't see that. | ||
| # the 'misc' is for 'error: Keywords must be strings' | ||
| expect = lazydf.collect(**final_polars_collect_kwargs) # type: ignore[misc, call-overload] | ||
| got = lazydf.collect(**final_cudf_collect_kwargs, engine=engine) # type: ignore[misc, call-overload] | ||
| expect = lazydf.collect(**cpu_kwargs) # type: ignore[misc, call-overload] | ||
| got = lazydf.collect(**gpu_kwargs, engine=engine) # type: ignore[misc, call-overload] | ||
| # In multi-rank SPMD mode each rank holds only its local slice; gather the | ||
| # full result on every rank so each rank can compare against the CPU result. | ||
| if ( | ||
|
|
@@ -158,102 +152,6 @@ def assert_ir_translation_raises(q: pl.LazyFrame, *exceptions: type[Exception]) | |
| raise AssertionError(f"Translation DID NOT RAISE {exceptions}") | ||
|
|
||
|
|
||
| def _process_kwargs( | ||
| collect_kwargs: CollectKwargs | None, | ||
| polars_collect_kwargs: CollectKwargs | None, | ||
| cudf_collect_kwargs: CollectKwargs | None, | ||
| ) -> tuple[CollectKwargs, CollectKwargs]: | ||
| if collect_kwargs is None: | ||
| collect_kwargs = {} | ||
| final_polars_collect_kwargs = collect_kwargs.copy() | ||
| final_cudf_collect_kwargs = collect_kwargs.copy() | ||
| if polars_collect_kwargs is not None: # pragma: no cover; not currently used | ||
| final_polars_collect_kwargs.update(polars_collect_kwargs) | ||
| if cudf_collect_kwargs is not None: # pragma: no cover; not currently used | ||
| final_cudf_collect_kwargs.update(cudf_collect_kwargs) | ||
| return final_polars_collect_kwargs, final_cudf_collect_kwargs | ||
|
Comment on lines
-161
to
-174
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inlining, the function is only called once now. |
||
|
|
||
|
|
||
| def assert_collect_raises( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inlining the function is simpler to understand IMO. We now just do: - assert_collect_raises(
- q,
- cudf_except=pl.exceptions.ComputeError,
- polars_except=pl.exceptions.InvalidOperationError,
- )
+ with pytest.raises(pl.exceptions.InvalidOperationError):
+ q.collect()
+ with pytest.raises(pl.exceptions.ComputeError):
+ q.collect(engine=pl.GPUEngine(executor="in-memory", raise_on_fail=True)) |
||
| lazydf: pl.LazyFrame, | ||
| *, | ||
| polars_except: type[Exception] | tuple[type[Exception], ...], | ||
| cudf_except: type[Exception] | tuple[type[Exception], ...], | ||
| collect_kwargs: CollectKwargs | None = None, | ||
| polars_collect_kwargs: CollectKwargs | None = None, | ||
| cudf_collect_kwargs: CollectKwargs | None = None, | ||
| ) -> None: | ||
| """ | ||
| Assert that collecting the result of a query raises the expected exceptions. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| lazydf | ||
| frame to collect. | ||
| collect_kwargs | ||
| Common keyword arguments to pass to collect for both polars CPU and | ||
| cudf-polars. | ||
| Useful for controlling optimization settings. | ||
| polars_except | ||
| Exception or exceptions polars CPU is expected to raise. If | ||
| an empty tuple ``()``, CPU is expected to succeed without raising. | ||
| cudf_except | ||
| Exception or exceptions polars GPU is expected to raise. If | ||
| an empty tuple ``()``, GPU is expected to succeed without raising. | ||
| collect_kwargs | ||
| Common keyword arguments to pass to collect for both polars CPU and | ||
| cudf-polars. | ||
| Useful for controlling optimization settings. | ||
| polars_collect_kwargs | ||
| Keyword arguments to pass to collect for execution on polars CPU. | ||
| Overrides kwargs in collect_kwargs. | ||
| Useful for controlling optimization settings. | ||
| cudf_collect_kwargs | ||
| Keyword arguments to pass to collect for execution on cudf-polars. | ||
| Overrides kwargs in collect_kwargs. | ||
| Useful for controlling optimization settings. | ||
|
|
||
| Returns | ||
| ------- | ||
| None | ||
| If both sides raise the expected exceptions. | ||
|
|
||
| Raises | ||
| ------ | ||
| AssertionError | ||
| If either side did not raise the expected exceptions. | ||
| """ | ||
| final_polars_collect_kwargs, final_cudf_collect_kwargs = _process_kwargs( | ||
| collect_kwargs, polars_collect_kwargs, cudf_collect_kwargs | ||
| ) | ||
|
|
||
| try: | ||
| lazydf.collect(**final_polars_collect_kwargs) # type: ignore[misc, call-overload] | ||
| except polars_except: | ||
| pass | ||
| except Exception as e: | ||
| raise AssertionError( | ||
| f"CPU execution RAISED {type(e)}, EXPECTED {polars_except}" | ||
| ) from e | ||
| else: | ||
| if polars_except != (): | ||
| raise AssertionError(f"CPU execution DID NOT RAISE {polars_except}") | ||
|
|
||
| # TODO: https://github.com/rapidsai/cudf/issues/22346 | ||
| engine = GPUEngine(executor="in-memory", raise_on_fail=True) | ||
| try: | ||
| lazydf.collect(**final_cudf_collect_kwargs, engine=engine) # type: ignore[misc, call-overload] | ||
| except cudf_except: | ||
| pass | ||
| except Exception as e: | ||
| raise AssertionError( | ||
| f"GPU execution RAISED {type(e)}, EXPECTED {cudf_except}" | ||
| ) from e | ||
| else: | ||
| if cudf_except != (): | ||
| raise AssertionError(f"GPU execution DID NOT RAISE {cudf_except}") | ||
|
|
||
|
|
||
| def _resolve_sink_format(path: Path) -> str: | ||
| """Returns valid sink format for assert utilities.""" | ||
| suffix = path.suffix.lower() | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never used