Copilot commented on code in PR #37396:
URL: https://github.com/apache/superset/pull/37396#discussion_r2723074394
##########
superset/utils/core.py:
##########
@@ -1756,6 +1756,42 @@ def extract_dataframe_dtypes(
return generic_types
+def extract_display_labels(
+ label_map: dict[str, list[str]],
+ colnames: list[str],
+ datasource: Explorable | None = None,
+) -> list[str]:
+ """Extract display labels for a list of column names based on a label map
+ and an optional datasource.
+ """
+ if not colnames:
+ return []
+
+ # Build column -> label mapping (skip self-references)
+ columns_to_label = {}
+ if label_map:
+ for label, cols in label_map.items():
+ for col in cols:
+ if label != col and col not in columns_to_label:
+ columns_to_label[col] = label
+
+ # Build column -> object mapping
+ columns_by_name: dict[str, Any] = {}
+ if datasource:
+ for column in datasource.columns:
+ if isinstance(column, dict):
+ if column_name := column.get("column_name"):
+ columns_by_name[column_name] = column
+ else:
+ columns_by_name[column.column_name] = column
+
+ return [
+ columns_to_label.get(col)
+ or (get_column_name(columns_by_name[col]) if col in columns_by_name
else col)
+ for col in colnames
+ ]
Review Comment:
`extract_display_labels` introduces new backend behavior that affects API
responses (`collabels`) but doesn’t have any Python test coverage. There are
existing tests for nearby utilities in `tests/integration_tests/utils_tests.py`
(e.g. `test_extract_dataframe_dtypes`), so adding a test for
`extract_display_labels` would help lock in expected label-map/datasource
fallback behavior and prevent regressions.
##########
superset/utils/core.py:
##########
@@ -1756,6 +1756,42 @@ def extract_dataframe_dtypes(
return generic_types
+def extract_display_labels(
+ label_map: dict[str, list[str]],
+ colnames: list[str],
+ datasource: Explorable | None = None,
+) -> list[str]:
+ """Extract display labels for a list of column names based on a label map
+ and an optional datasource.
+ """
+ if not colnames:
+ return []
+
+ # Build column -> label mapping (skip self-references)
+ columns_to_label = {}
+ if label_map:
+ for label, cols in label_map.items():
+ for col in cols:
+ if label != col and col not in columns_to_label:
+ columns_to_label[col] = label
+
+ # Build column -> object mapping
+ columns_by_name: dict[str, Any] = {}
+ if datasource:
+ for column in datasource.columns:
+ if isinstance(column, dict):
+ if column_name := column.get("column_name"):
+ columns_by_name[column_name] = column
+ else:
+ columns_by_name[column.column_name] = column
+
+ return [
+ columns_to_label.get(col)
+ or (get_column_name(columns_by_name[col]) if col in columns_by_name
else col)
+ for col in colnames
+ ]
Review Comment:
`extract_display_labels` can raise `ValueError("Missing label")` when
`datasource.columns` contains dict-style column metadata (eg `{"column_name":
..., "is_dttm": ...}`) because it calls `get_column_name()` on those dicts, and
`get_column_name()` only supports dicts with `label`/`sqlExpression` keys.
Please make the datasource fallback extraction tolerant of dict columns (eg
prefer `verbose_name`/`column_name`/`name` keys) so this never throws for valid
datasources.
##########
superset-frontend/src/explore/components/DataTableControl/index.tsx:
##########
@@ -222,6 +222,31 @@ const DataTableTemporalHeaderCell = ({
);
};
+const DataTableHeaderCell = ({
+ columnName,
+ columnLabel,
+}: {
+ columnName: string;
+ columnLabel?: string;
+}) => {
+ // Use label if provided, otherwise use column name
+ // as header
+ const displayText = columnLabel || columnName;
+ if (columnLabel && columnLabel !== columnName) {
+ return (
+ <Popover
+ content={`Column name: ${columnName}`}
+ placement="bottomLeft"
+ arrow={{ pointAtCenter: true }}
+ >
+ <span>{displayText}</span>
+ </Popover>
+ );
Review Comment:
New UI text in the Popover (`"Column name: ..."`) isn’t wrapped in `t()` for
i18n, while the rest of this module consistently localizes user-visible
strings. Please translate this string (and keep the column name interpolated)
so it’s included in Superset’s localization workflow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
