TST: Removed import of itertools #32364
Conversation
| @@ -1,7 +1,6 @@ | |||
| import builtins | |||
| import datetime as dt | |||
| import datetime | |||
There was a problem hiding this comment.
can you separate this change into another PR for discussion. (keep this PR as removing itertools)
The reason is that we only have 7 occurrences of import datetime as dt in the codebase and there is some discrepancy between which datetime of datetime.datetime is aliased as dt
The more common pattern is like from datetime import date, datetime, time, timedelta, tzinfo
probably best with these cleanups, is to pick one type of change at a time.
There was a problem hiding this comment.
probably best with these cleanups, is to pick one type of change at a time.
and then after appropriate discussion can apply the changes across the complete codebase.
| @@ -1296,36 +1307,32 @@ def __eq__(self, other): | |||
| # -------------------------------- | |||
|
|
|||
|
|
|||
| def test_size(df): | |||
| grouped = df.groupby(["A", "B"]) | |||
| @pytest.mark.parametrize("group_key", ["A", "B", ["A", "B"]]) | |||
There was a problem hiding this comment.
I think you could just call this by to mirror the parameter name.
| grouped = df.groupby(["A", "B"]) | ||
| @pytest.mark.parametrize("group_key", ["A", "B", ["A", "B"]]) | ||
| def test_size(df, group_key): | ||
| grouped = df.groupby(group_key) |
There was a problem hiding this comment.
maybe
| grouped = df.groupby(group_key) | |
| grouped = df.groupby(by=by) |
if you think the parameterisation is not clear.
| result = grouped.size() | ||
| for key, group in grouped: | ||
| assert result[key] == len(group) | ||
| @pytest.mark.parametrize("key", ["A", "B", ["A", "B"]]) |
There was a problem hiding this comment.
again use by or be consistent between the two tests.
| for key, group in grouped: | ||
| assert result[key] == len(group) | ||
| @pytest.mark.parametrize("key", ["A", "B", ["A", "B"]]) | ||
| @pytest.mark.parametrize("sort", [True, False]) |
There was a problem hiding this comment.
This brings the total number of sort parameterizations for [True, False] to 12. so maybe now worth considering creating a fixture for this. (as a follow-up)
There was a problem hiding this comment.
I think that is already a fixture for sort but it's only for [None, False] IIRC.
There was a problem hiding this comment.
we can use different fixtures with the same name. The fixture used depends on the directory structure.
we have the relevant fixture in pandas\tests\reshape\test_concat.py, so just needs to be moved.
The sort fixture in pandas\tests\indexes\conftest.py has a caution note. maybe worth adding something like that.
BUT, if you do this, do in a follow-up, not here.
|
Thanks @MomIsBestFriend |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff