-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix test_merge_by_index_patterns
for pandas
2.0
#9930
Conversation
test_merge_by_index_patterns
for pandas
2.0 compatibility
a68c688
to
ec128b9
Compare
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 for your work here @j-bennet. It looks like there are other similar failures (e.g. dask/dataframe/tests/test_multi.py::test_merge_by_multiple_columns
, dask/dataframe/tests/test_shuffle.py::test_set_index_overlap_2
, etc) that are still happening. Would you like to handle those here, or in a follow-up?
Good catch, yes, they look like the same error. I'll include them in the PR. |
Didn't find a good way to fix |
f5dae85
to
a8374fd
Compare
The remaining test failures here are flaky tests:
|
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 @j-bennet. Overall this looks reasonable. Are these changes more of a workaround and we should follow-up to make the index match, or is this a good long-term solution?
It's a workaround. I created an issue to fix this properly: |
Great, thanks @j-bennet |
test_merge_by_index_patterns
for pandas
2.0 compatibilitytest_merge_by_index_patterns
for pandas
2.0
Fixes the following failures in CI with pandas 2.0:
The failure is related to the following change in 2.0:
https://pandas.pydata.org/docs/dev/whatsnew/v2.0.0.html#empty-dataframes-series-will-now-default-to-have-a-rangeindex
In this test, we're already applying a workaround to set
dtype
on returnedpandas
dataframes, becausepandas
doesn't set it on empty dataframes. But with 2.0, it looks like we needed to apply this in more places. Not sure why this changed.pre-commit run --all-files