-
Notifications
You must be signed in to change notification settings - Fork 3
Add count to UI #725
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
base: main
Are you sure you want to change the base?
Add count to UI #725
Conversation
|
|
||
| columns_icon = icon_svg("table-columns") | ||
| groups_icon = icon_svg("table") | ||
| row_counts_icon = icon_svg("hashtag") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this should be it's own panel, do any of the other icons seem better?
| DP Statistics for `data_col`, grouped by `grouping_col`, without counts | ||
| >>> print(plan.to_stem()) | ||
| dp_statistics_for_data_col_grouped_by_grouping_col | ||
| dp_statistics_for_data_col_grouped_by_grouping_col_without_counts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the without_counts too much?
|
Suggestion from Ellen: Always do count, and explain in the notebook that it is optional. Less UI clutter, and simpler code generation, and probably less confusing to users. |
First, just want to allow counts to be manually turned on. After this, will turn on counts in the AnalysisPlan (even if not selected in UI) when a mean is requested, so we only need to release the DP count once.
For reviewer:
If anything feels sketchy here, one possibility would be to keep a lot of the tooling changes, which are unequivocally improvements, and then try a different approach for the count analysis.