-
-
Notifications
You must be signed in to change notification settings - Fork 18.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
Shadimrad
commented
Jul 14, 2022
•
edited by mroeschke
Loading
edited by mroeschke
- closes columns comparison with Enum column name triggers TypeError #22551
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 |
@@ -20,3 +22,16 @@ def test_get_value(self, float_frame): | |||
result = float_frame._get_value(idx, col) | |||
expected = float_frame[col][idx] | |||
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! |