Skip to content
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

adding mode as aggfunc #194

Merged
merged 5 commits into from
Feb 20, 2024
Merged

adding mode as aggfunc #194

merged 5 commits into from
Feb 20, 2024

Conversation

mahaalbashir
Copy link
Contributor

No description provided.

@mahaalbashir mahaalbashir linked an issue Jan 30, 2024 that may be closed by this pull request
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (62e43d4) 99.62% compared to head (b8faef1) 99.63%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #194   +/-   ##
=======================================
  Coverage   99.62%   99.63%           
=======================================
  Files           9        9           
  Lines        1070     1086   +16     
=======================================
+ Hits         1066     1082   +16     
  Misses          4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

line 1102 where you see if all the values in a series are unique.
sorting might take a while - could you just use https://pandas.pydata.org/docs/reference/api/pandas.Series.nunique.html

Copy link
Contributor

@jim-smith jim-smith Jan 30, 2024

Choose a reason for hiding this comment

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

Looking at the levels of indentation and the nu,mbersof line in this method near where you calculate threshold masks etc., (lines 742 onwards) , I wonder if it is time to refactor some of this code into a series of functions called something like get_mask_for_aggfuncc()

@jim-smith jim-smith merged commit 4c137fc into main Feb 20, 2024
6 checks passed
@jim-smith jim-smith deleted the adding_mode branch February 20, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement more aggregation functions (Mode/Median)
2 participants