Skip to content

crosstab: speedup dask case #596

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

Merged
merged 3 commits into from
Dec 18, 2021
Merged

crosstab: speedup dask case #596

merged 3 commits into from
Dec 18, 2021

Conversation

thuydotm
Copy link
Contributor

No description provided.

@thuydotm thuydotm added the ready to merge PR is ready to merge label Dec 14, 2021
@thuydotm thuydotm requested a review from ianthomas23 December 14, 2021 10:55
@thuydotm thuydotm force-pushed the crosstab_speedup_3d branch from 07951c7 to 40f478f Compare December 15, 2021 12:56
Copy link
Contributor

@ianthomas23 ianthomas23 left a comment

Choose a reason for hiding this comment

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

Just a few comments.

Also, _sort_values function at line 499 is no longer used so can be removed.

@ianthomas23
Copy link
Contributor

In future, it would be really good if performance improvement PRs could include an ASV benchmark as the output of that would show a side-by-side comparison of the performance before and after the changes.

@thuydotm thuydotm merged commit 6bdb7cb into master Dec 18, 2021
@thuydotm thuydotm deleted the crosstab_speedup_3d branch December 23, 2021 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge PR is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants