-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
TST: Test for the Enum triggering TypeError (#22551 issue) #47715
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
Conversation
|
Hello @Shadimrad! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2022-07-16 02:19:24 UTC |
| assert result == expected | ||
|
|
||
|
|
||
| def test_enum_value(): |
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.
- Could you name this test
test_enum_column_equality - Could you move this to
pandas/tests/frame/test_arithmetic.py? - Also below could you structure the test like
result = q1[Cols.col1] == q2[Cols.col1]
expected = ...
tm.assert_equal(result, expected)
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.
Sure! Thank you for your guide.
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.
@mroeschke by result = q1[Cols.col1] == q2[Cols.col1] do you mean that I remove the .all() method as well?
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.
Correct
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.
The changes in this file can be removed now.
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.
If you inspect result, it's a Series
In [1]: from enum import Enum
In [2]: Cols = Enum("Cols", "col1 col2")
...:
...: q1 = DataFrame({Cols.col1: [1, 2, 3]})
...: q2 = DataFrame({Cols.col1: [1, 2, 3]})
...:
...: result = q1[Cols.col1] == q2[Cols.col1]
In [3]: result
Out[3]:
0 True
1 True
2 True
Name: Cols.col1, dtype: bool
So expected should also be a Series
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.
@mroeschke
Right. but when I created the series and tried to compare,
data = np.array([True, True, True])
expected = pd.Series(data)
checking if expected == result gave me the Error that the truth value of the Series is ambiguous, that is why. Unless tm.assert_equal accounts for that. update: it did not.
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.
Yup, tm.assert_equal will check the values and the aspects of the Series
In [1]: from enum import Enum
In [2]: Cols = Enum("Cols", "col1 col2")
...:
...: q1 = DataFrame({Cols.col1: [1, 2, 3]})
...: q2 = DataFrame({Cols.col1: [1, 2, 3]})
...:
...: result = q1[Cols.col1] == q2[Cols.col1]
In [3]: result
Out[3]:
0 True
1 True
2 True
Name: Cols.col1, dtype: bool
In [4]: import pandas._testing as tm
In [5]: epxected = pd.Series([True, True, True], name=Cols.col1)
In [6]: tm.assert_series_equal(result, epxected)
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.
Update: changing it to tm.assert_series_equal(result, expected) , hoping it works.
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.
Oh, I've just seen your reply. There was a syntax issue that I fixed. pd.Series now replaced with Series which hopefully should be working now. Thank you for your guidance as well.
|
I believe it's done! @mroeschke |
|
Thanks, great job @Shadimrad! |
Uh oh!
There was an error while loading. Please reload this page.