-
Notifications
You must be signed in to change notification settings - Fork 913
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
Add conda recipe for cudf-polars #17037
Add conda recipe for cudf-polars #17037
Conversation
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.
Thanks Bradley!
Can we move the configuration of coverage? Otherwise looks good to me, will let others chime in on the packaging side of things.
python/cudf_polars/.coveragerc
Outdated
# Configuration file for Python coverage tests | ||
[run] | ||
source = cudf_polars |
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.
We already do some configuration of coverage in pyproject.toml. Can this configuration also go there (I suppose in a [tool.coverage.run]
section?
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.
We may not even need a separate section for this. I pushed 1a2be0d to try using pyproject.toml
without defining the [tool.coverage.run]
source
.
@@ -96,7 +96,7 @@ def test_bool_agg(agg, request): | |||
assert_gpu_result_equal(q, check_exact=False) | |||
|
|||
|
|||
@pytest.mark.parametrize("cum_agg", expr.UnaryFunction._supported_cum_aggs) | |||
@pytest.mark.parametrize("cum_agg", sorted(expr.UnaryFunction._supported_cum_aggs)) |
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.
Thanks, good spot!
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.
For reference, this was necessary to make multi-worker testing operate correctly. The frozenset
had a different order on each worker, so pytest failed because the test collection didn't agree across all workers.
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.
Thanks, approving the python changes. And I had some non-blocking questions on the ci changes.
Co-authored-by: Matthew Murray <[email protected]>
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.
No other comments from me, looks good.
I confirmed that coverage is working:
|
/merge |
Follow-up to rapidsai/cudf#17037. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - James Lamb (https://github.com/jameslamb) URL: #732
Description
This PR adds conda recipes for
cudf-polars
. This is needed to getcudf-polars
into RAPIDS Docker containers and therapids
metapackage.Closes #16816.
Checklist