-
Notifications
You must be signed in to change notification settings - Fork 847
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 support for pandas-2.2
in cudf
#15100
Changes from 8 commits
5262a4e
1e77124
32c1c0f
6daba82
00f1bbe
f42676d
a6c78ec
ec80e9a
2c00c59
04309a6
d15323a
7fcc521
7da7100
82b0644
391b602
1b8a1e5
ecd9221
fa72dd8
65c8652
07d57f6
adcd271
3efb4f8
cae3b0d
ffa1edc
d50ed04
415de9b
ca9933d
340a4de
65bba60
a926ad1
328bc95
4f76e9f
62cf147
fb8924b
8159ac2
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 |
---|---|---|
|
@@ -9,7 +9,6 @@ | |
PANDAS_GE_201 = PANDAS_VERSION >= version.parse("2.0.1") | ||
PANDAS_GE_210 = PANDAS_VERSION >= version.parse("2.1.0") | ||
PANDAS_GE_214 = PANDAS_VERSION >= version.parse("2.1.4") | ||
PANDAS_GE_220 = PANDAS_VERSION >= version.parse("2.2.0") | ||
PANDAS_LT_203 = PANDAS_VERSION < version.parse("2.0.3") | ||
PANDAS_GE_220 = PANDAS_VERSION >= version.parse("2.2.0") | ||
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. Can we sort these, so we have all the 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. Yup, I'll do that in a follow-up pr. Potentially there is still some cleanup to be done. |
||
PANDAS_LT_300 = PANDAS_VERSION < version.parse("3.0.0") |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
ScalarLike, | ||
) | ||
from cudf.api.types import is_datetime64_dtype, is_scalar, is_timedelta64_dtype | ||
from cudf.core._compat import PANDAS_GE_200, PANDAS_GE_220 | ||
from cudf.core._compat import PANDAS_GE_220 | ||
from cudf.core.buffer import Buffer, cuda_array_interface_wrapper | ||
from cudf.core.column import ColumnBase, as_column, column, string | ||
from cudf.core.column.timedelta import _unit_to_nanoseconds_conversion | ||
|
@@ -324,17 +324,8 @@ def to_pandas( | |
# `copy=True` workaround until following issue is fixed: | ||
# https://issues.apache.org/jira/browse/ARROW-9772 | ||
|
||
if PANDAS_GE_200: | ||
host_values = self.to_arrow() | ||
else: | ||
# Pandas<2.0 supports only `datetime64[ns]`, hence the cast. | ||
host_values = self.astype("datetime64[ns]").to_arrow() | ||
|
||
# Pandas only supports `datetime64[ns]` dtype | ||
# and conversion to this type is necessary to make | ||
# arrow to pandas conversion happen for large values. | ||
return pd.Series( | ||
host_values, | ||
self.to_arrow(), | ||
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. Just in noting in the future we should pass pandas a numpy array instead of a pyarrow array and |
||
copy=True, | ||
dtype=self.dtype, | ||
index=index, | ||
|
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.
Is this the right pinning, or should we go with
<2.3.0dev0
? Do we expect patch releases to break us?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 feel various fixes are going into
2.2.1
that may have an impact on us, but would defer to @mroeschke on that.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.
Yeah I fixed various issues @galipremsagar found in pandas 2.2.1 (coming out next week) which there are xfails for so the update will break the test suite. We should probably relax the pin in a follow up PR when that's out