-
Notifications
You must be signed in to change notification settings - Fork 915
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
Changes to support Numpy >= 1.24 #13325
Changes from 18 commits
4ad5832
809cc6e
4e4ad82
f8da5dc
e79b8cd
b94427f
66365b3
ba3ede7
f884eb7
42d6a74
0eb153c
d75986b
2dfb939
860f439
8221628
abdc026
37c9514
c4bccf4
e776efc
0bff932
e4b43ac
9a68604
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -398,8 +398,8 @@ def test_column_view_string_slice(slc): | |
cudf.core.column.as_column([], dtype="uint8"), | ||
), | ||
( | ||
cp.array([453], dtype="uint8"), | ||
cudf.core.column.as_column([453], dtype="uint8"), | ||
cp.array([255], dtype="uint8"), | ||
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. I guess it doesn't matter what value we choose here? Just wondering if it's important to use 453-256. 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. I don't think it matters. I use |
||
cudf.core.column.as_column([255], dtype="uint8"), | ||
), | ||
], | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# Copyright (c) 2021-2022, NVIDIA CORPORATION. | ||
# Copyright (c) 2021-2023, NVIDIA CORPORATION. | ||
|
||
import numpy as np | ||
import pandas as pd | ||
|
@@ -194,6 +194,7 @@ def test_to_numeric_downcast_int(data, downcast): | |
assert_eq(expected, got) | ||
|
||
|
||
@pytest.mark.filterwarnings("ignore:invalid value encountered in cast") | ||
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. Instead of applying this to the whole test, can we just wrap the Also, should we be handling the warning conditionally? i.e. I assuming this happens when trying to downcast a signed to an unsigned type or something? |
||
@pytest.mark.parametrize( | ||
"data", | ||
[ | ||
|
@@ -223,6 +224,7 @@ def test_to_numeric_downcast_float(data, downcast): | |
assert_eq(expected, got) | ||
|
||
|
||
@pytest.mark.filterwarnings("ignore:invalid value encountered in cast") | ||
@pytest.mark.parametrize( | ||
"data", | ||
[ | ||
|
@@ -245,6 +247,7 @@ def test_to_numeric_downcast_large_float(data, downcast): | |
assert_eq(expected, got) | ||
|
||
|
||
@pytest.mark.filterwarnings("ignore:overflow encountered in cast") | ||
@pytest.mark.parametrize( | ||
"data", | ||
[ | ||
|
@@ -325,6 +328,7 @@ def test_to_numeric_downcast_string_float(data, downcast): | |
assert_eq(expected, got) | ||
|
||
|
||
@pytest.mark.filterwarnings("ignore:overflow encountered in cast") | ||
@pytest.mark.parametrize( | ||
"data", | ||
[ | ||
|
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.
This hack will go away
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.
I think it can be removed from this PR itself. Now that we've verified numpy 1.24 support, I would recommend removing the numba-related changes in this PR that you're using in order to allow running numba 0.57 (which is necessary to use numpy 1.24). We'll still get it tested because our CUDA 12 wheel builds will patch us to use 0.57 anyway (but with CUDA 12 we don't use cubinlinker/ptxcompiler so we don't need any edits for those). Then when we bump our numba to 0.57 tests should pass thanks to this PR.
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.
I have one other question on this PR, would wait to make changes here until everything else is resolved in case you need to run more tests locally.