-
Notifications
You must be signed in to change notification settings - Fork 811
add categories subtext and add min-width for each bar #7042
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,7 @@ export const TableColumnSummary = <TData, TValue>({ | |
| } | ||
|
|
||
| return ( | ||
| <div className="flex justify-between w-full px-2 whitespace-pre"> | ||
| <div className="flex justify-between w-full px-2 whitespace-pre -mt-1.5"> | ||
| <span>{renderDate(stats.min, type)}</span> | ||
| {stats.min === stats.max ? null : ( | ||
| <span>-{renderDate(stats.max, type)}</span> | ||
|
|
@@ -145,15 +145,20 @@ export const TableColumnSummary = <TData, TValue>({ | |
| return null; | ||
| case "time": | ||
| return null; | ||
| case "string": | ||
| if (!spec) { | ||
| return ( | ||
| <div className="flex flex-col whitespace-pre"> | ||
| <span>unique: {prettyNumber(stats.unique, locale)}</span> | ||
| </div> | ||
| ); | ||
| case "string": { | ||
| const lessThan5Categories = | ||
| typeof stats.unique === "number" && stats.unique <= 5; | ||
| // Number of categories will be easily visible from the chart, so don't show it here. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think its that easy to parse number of unique values, i would have to count each bar chart |
||
| if (spec && lessThan5Categories) { | ||
| return null; | ||
| } | ||
| return null; | ||
|
|
||
| return ( | ||
| <div className="flex flex-col whitespace-pre -mt-1.5"> | ||
| <span>categories: {prettyNumber(stats.unique, locale)}</span> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe |
||
| </div> | ||
| ); | ||
| } | ||
| case "unknown": | ||
| return ( | ||
| <div className="flex flex-col whitespace-pre"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -990,7 +990,10 @@ def _get_column_summaries( | |||||||||
| ): | ||||||||||
| try: | ||||||||||
| val_counts = self._get_value_counts( | ||||||||||
| column, DEFAULT_VALUE_COUNTS_SIZE, total_rows | ||||||||||
| column=column, | ||||||||||
| size=DEFAULT_VALUE_COUNTS_SIZE, | ||||||||||
| total_rows=total_rows, | ||||||||||
| others_threshold=1, | ||||||||||
| ) | ||||||||||
| if len(val_counts) > 0: | ||||||||||
| value_counts[column] = val_counts | ||||||||||
|
|
@@ -1043,7 +1046,12 @@ def _get_column_summaries( | |||||||||
| ) | ||||||||||
|
|
||||||||||
| def _get_value_counts( | ||||||||||
| self, column: ColumnName, size: int, total_rows: int | ||||||||||
| self, | ||||||||||
| *, | ||||||||||
| column: ColumnName, | ||||||||||
| size: int, | ||||||||||
| total_rows: int, | ||||||||||
| others_threshold: int | None = None, | ||||||||||
| ) -> list[ValueCount]: | ||||||||||
| """Get value counts for a column. The last item will be 'others' with the count of remaining | ||||||||||
| unique values. If there are only unique values, we return 'unique values' instead. | ||||||||||
|
|
@@ -1052,6 +1060,8 @@ def _get_value_counts( | |||||||||
| column (ColumnName): The column to get value counts for. | ||||||||||
| size (int): The number of value counts to return. | ||||||||||
| total_rows (int): The total number of rows in the table. | ||||||||||
| others_threshold (int | None): The threshold for values to be considered 'others'. | ||||||||||
| If a value has a count less than or equal to this threshold, it will be considered 'others'. | ||||||||||
|
Comment on lines
+1063
to
+1064
|
||||||||||
| others_threshold (int | None): The threshold for values to be considered 'others'. | |
| If a value has a count less than or equal to this threshold, it will be considered 'others'. | |
| others_threshold (int | None): The threshold for filtering values into others. | |
| Values with counts at or below this threshold will be excluded from the individual value list and grouped into others. |
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.
[nitpick] The magic number 0.02 should be defined as a named constant at the module or class level for better maintainability and reusability. Consider defining it as
const MIN_BAR_WIDTH_PERCENT = 0.02outside the function scope.