Skip to content

Conversation

@jessica-writes-code
Copy link
Contributor

@jessica-writes-code jessica-writes-code commented Dec 26, 2022

Add tests to verify that the Series method isin returns consistent results when passed iterables with different dtypes. (See #50234.)

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

This is off to a good start!

@MarcoGorelli MarcoGorelli added the Needs Tests Unit test(s) needed to prevent regressions label Dec 27, 2022
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

couple more minor comments, then looks good to me

@phofl any further comments?


@pytest.mark.parametrize(
"data",
[([1, 2, 3]), ([1.0, 2.0, 3.0])],
Copy link
Member

Choose a reason for hiding this comment

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

looks like you've got some extra brackets (I'm surprised none of the linters caught this)

Suggested change
[([1, 2, 3]), ([1.0, 2.0, 3.0])],
[[1, 2, 3], [1.0, 2.0, 3.0]],

# GH 50234

ser = Series(data)
result = ser.isin(i for i in [1, 2])
Copy link
Member

Choose a reason for hiding this comment

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

sorry, I didn't catch this the first time round - doesn't the original issue have (i for i in [1.0, 2.0])?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, we should parametrize over both, e.g. [1, 2] and [1.0, 2.0]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Updated parameters accordingly.

@MarcoGorelli MarcoGorelli added this to the 2.0 milestone Dec 27, 2022
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Nice, thanks @jessica-writes-code ! Looks good to me pending green

@phofl phofl added the Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff label Dec 27, 2022
@phofl phofl merged commit dd8b721 into pandas-dev:main Dec 27, 2022
@phofl
Copy link
Member

phofl commented Dec 27, 2022

Thx @jessica-writes-code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff Needs Tests Unit test(s) needed to prevent regressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: pd.Series.isin returns inconsistent results for a passed iterable depending on the dtype

3 participants