Fix #28552 Add test for DataFrame min/max preserve timezone #33905
Conversation
|
Hello @dair-targ! 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 2020-05-12 11:57:56 UTC |
pandas/tests/frame/test_timezones.py
Outdated
| "initial", | ||
| ["2018-10-08 13:36:45+00:00", "2018-10-08 13:36:45+03:00"], # Non-UTC timezone | ||
| ) | ||
| @pytest.mark.parametrize("method", [DataFrame.min, DataFrame.max]) |
There was a problem hiding this comment.
Could you use the strings ['min', 'max'] here, and then below you can do getattr(df, method)(axis=1)
pandas/tests/frame/test_timezones.py
Outdated
| # When | ||
| actual_df = method(df, axis=1) | ||
| # Then | ||
| assert actual_df[0].tz == initial_dt.tz |
There was a problem hiding this comment.
Could you rename the results result and expected and compare 2 DataFrame or Series objects and use tm.assert_equal(result, expected)
There was a problem hiding this comment.
Addressed. Please note that min and max methods of the DataFrame both return Series, so I choose to use the tm.assert_series_equal instead in the same manner as it is being used in test_frame_no_datetime64_dtype.
…one after code review
|
@mroeschke thanks for review! I hope I've addressed all the issues - would you mind to review it again? |
jreback
added
Reshaping
|
thanks @dair-targ |
ghost
deleted the
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffHere's a test discussed in #28552.
meanand other aggregates except forminandmaxare not covered since they don't support datetimes - it's a nan being returned.