Skip to content

Cleaning up misc tests for pandas compat#8623

Merged
jsignell merged 2 commits intodask:mainfrom
jsignell:pandas-compat-misc
Jan 26, 2022
Merged

Cleaning up misc tests for pandas compat#8623
jsignell merged 2 commits intodask:mainfrom
jsignell:pandas-compat-misc

Conversation

@jsignell
Copy link
Member

This fixes:

  • dask/dataframe/tests/test_dataframe.py::test_replace
  • dask/dataframe/tests/test_dataframe.py::test_dataframe_mode
  • dask/dataframe/tests/test_reshape.py::test_get_dummies_kwargs

ds = dd.from_pandas(s, 2)
res = dd.get_dummies(ds, dummy_na=True)
assert_eq(res, exp)
tm.assert_index_equal(res.columns, pd.Index([1, 2, 3, 5, np.nan]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tm.assert_index_equal call fails with

E       AssertionError: Index are different
E
E       Index classes are different
E       [left]:  CategoricalIndex([1, 2, 3, 5, nan], categories=[1, 2, 3, 5], ordered=False, dtype='category')
E       [right]: Float64Index([1.0, 2.0, 3.0, 5.0, nan], dtype='float64')

but all the other tm.assert_index_equal calls in this test pass on pandas 1.4.0. Why are we removing the other asserts?

Alternatively, could we just update this line to use pd.Index([1, 2, 3, 5, np.nan], dtype=res.columns.dtype) instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index is already being checked in assert_eq. I imagine that this tests was just outdated.

@jsignell jsignell requested a review from jrbourbeau January 25, 2022 22:08
@jrbourbeau jrbourbeau mentioned this pull request Jan 26, 2022
3 tasks
@jsignell
Copy link
Member Author

Ok I think this one is good to go!

@jrbourbeau jrbourbeau mentioned this pull request Jan 26, 2022
3 tasks
@jsignell jsignell merged commit 7edfe24 into dask:main Jan 26, 2022
@jsignell jsignell deleted the pandas-compat-misc branch January 26, 2022 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants