Skip to content
This repository was archived by the owner on Feb 2, 2024. It is now read-only.

Conversation

@kozlov-alexey
Copy link
Contributor

No description provided.

@pep8speaks
Copy link

pep8speaks commented Feb 11, 2020

Hello @kozlov-alexey! 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-02-18 15:55:06 UTC

@kozlov-alexey kozlov-alexey force-pushed the feature/df_groupby_single_column branch from 6525865 to af914cb Compare February 12, 2020 14:15
pd.testing.assert_frame_equal(result, result_ref)

@skip_sdc_jit('Fails with old-pipeline from the start')
def test_dataframe_groupby_count_by_int(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe to combine tests test_dataframe_groupby_count_by_* via subTest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Comment on lines 161 to 165
'A': [2, 1, 2, 1, 2, 2, 1, 0, 3, 1, 3],
'B': np.arange(n, dtype=np.intp),
'C': np.arange(n, dtype=np.float_),
'D': [np.nan, 2., -1.3, np.nan, 3.5, 0, 10, 0.42, np.nan, -2.5, 23],
'E': [np.inf, 2., -1.3, -np.inf, 3.5, 0, 10, 0.42, np.nan, -2.5, 23]
Copy link
Contributor

Choose a reason for hiding this comment

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

The data is used in a lot of places. Wouldn't you like to move it to some common place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're probably right, I moved it to a global var.

Copy link
Contributor

@densmirn densmirn left a comment

Choose a reason for hiding this comment

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

If you will you could apply above comments in the separate PR.

@AlexanderKalistratov
Copy link
Collaborator

@kozlov-alexey conflicts.
Let's merge it as-is for now

@kozlov-alexey kozlov-alexey force-pushed the feature/df_groupby_single_column branch from 2e3b42c to 49ea6dd Compare February 18, 2020 10:55
@kozlov-alexey kozlov-alexey force-pushed the feature/df_groupby_single_column branch from 49ea6dd to 62d9b4d Compare February 18, 2020 15:54
@AlexanderKalistratov AlexanderKalistratov merged commit 3ca5a30 into IntelPython:master Feb 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants