codeant-ai-for-open-source[bot] commented on code in PR #44719: URL: https://github.com/apache/superset/pull/44719#discussion_r4113191296
########## superset/utils/excel_conditional.py: ########## @@ -0,0 +1,517 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Stamp Explore Table / Pivot Table v2 highlights onto an Excel workbook. + +Explore paints matching cells in the browser. Chart XLSX download is written +in ``QueryContextProcessor.get_data`` before client post-processing, so this +module is applied there as well as on the reports path. + +Rules from ``form_data["conditional_formatting"]`` become native Excel +conditional formatting (CellIs / formula / color scale / data bar). When that +list is empty and ``show_cell_bars`` is on, numeric columns get data bars so +the download matches the Table chart's default gradient. +""" + +from __future__ import annotations + +import io +from typing import Any, Mapping, Optional + +import pandas as pd +from openpyxl import load_workbook +from openpyxl.formatting.rule import ( + CellIsRule, + ColorScaleRule, + DataBarRule, + FormulaRule, +) +from openpyxl.styles import Font, PatternFill +from openpyxl.utils import get_column_letter +from openpyxl.workbook import Workbook +from openpyxl.worksheet.worksheet import Worksheet + +from superset.constants import SHOW_VALUES_AS_PERCENT_MODES +from superset.utils.excel_display import ( + apply_column_display, + refresh_sheet_bounds, + styles_from_pivot_form_data, + styles_from_table_form_data, +) + +# Theme tokens used by Table / Pivot Table v2 pickers, plus CSS names. +_NAMED_COLORS = { + "success": "52C41A", + "warning": "FAAD14", + "error": "FF4D4F", + "red": "FF4D4F", + "green": "52C41A", + "blue": "1890FF", + "yellow": "FAAD14", + "orange": "FA8C16", + "purple": "722ED1", + "cyan": "13C2C2", + "colorsuccess": "52C41A", + "colorwarning": "FAAD14", + "colorerror": "FF4D4F", + "colorsuccessbg": "F6FFED", + "colorwarningbg": "FFFBE6", + "colorerrorbg": "FFF2F0", +} + +_CELL_IS_OPERATORS = { + ">": "greaterThan", + "<": "lessThan", + ">=": "greaterThanOrEqual", + "<=": "lessThanOrEqual", + "=": "equal", + "==": "equal", + "!=": "notEqual", + "≠": "notEqual", + "≥": "greaterThanOrEqual", + "≤": "lessThanOrEqual", +} + +_RANGE_OPERATORS = { + "< x <": (">", "<"), + "< x ≤": (">", "<="), + "≤ x <": (">=", "<"), + "≤ x ≤": (">=", "<="), +} + +_DEFAULT_RULE_COLOR = _NAMED_COLORS["success"] +_DATA_BAR_POSITIVE = "63BE7B" +_SCALE_LOW = "FFFFFF" +_OBJECT_CELL_BAR = "CELL_BAR" +_OBJECT_TEXT = "TEXT_COLOR" +# First non-blank cells used to decide whether a column is numeric for +# default ``show_cell_bars``. Scanning the whole sheet is unnecessary for +# typical Table downloads; blank leading rows still skip until a value. +_NUMERIC_SAMPLE_ROWS = 20 + + +def _hex_rgb(color: Any) -> Optional[str]: + """Normalize a picker payload to a 6-digit RGB hex string.""" + if isinstance(color, dict): + hex_value = color.get("hex") + if isinstance(hex_value, str): + return _hex_rgb(hex_value) + red, green, blue = color.get("r"), color.get("g"), color.get("b") + if None not in (red, green, blue): + return f"{int(red):02X}{int(green):02X}{int(blue):02X}" + return None + if not isinstance(color, str) or not color: + return None + token = color.strip().lstrip("#") + named = _NAMED_COLORS.get(token.lower().replace("_", "").replace("-", "")) + if named: + return named + if len(token) == 3 and all(ch in "0123456789abcdefABCDEF" for ch in token): + return "".join(ch * 2 for ch in token).upper() + if len(token) >= 6 and all(ch in "0123456789abcdefABCDEF" for ch in token[:6]): + return token[:6].upper() + return None + + +def _rule_color(rule: dict[str, Any]) -> str: + return _hex_rgb(rule.get("colorScheme")) or _DEFAULT_RULE_COLOR + + +def _as_number(value: Any) -> Optional[float]: + if isinstance(value, bool) or value in (None, ""): + return None + if isinstance(value, (int, float)): + return float(value) + if isinstance(value, str): + try: + return float(value.replace(",", "")) + except ValueError: + return None + return None + + +def _value_matches_rule(value: Any, rule: dict[str, Any]) -> bool: + """Whether a cell should receive a CELL_BAR rule (Explore comparator).""" + if value in (None, ""): + return False + operator = rule.get("operator") + if operator in (None, "None", ""): + return True + number = _as_number(value) + if operator in _CELL_IS_OPERATORS: + target = rule.get("targetValue") + target_number = _as_number(target) + if number is not None and target_number is not None: + compare = _CELL_IS_OPERATORS[operator] + if compare == "greaterThan": + return number > target_number + if compare == "lessThan": + return number < target_number + if compare == "greaterThanOrEqual": + return number >= target_number + if compare == "lessThanOrEqual": + return number <= target_number + if compare == "equal": + return number == target_number + if compare == "notEqual": + return number != target_number + return str(value) == str(target) + if operator in _RANGE_OPERATORS: + left_op, right_op = _RANGE_OPERATORS[operator] + left = _as_number(rule.get("targetValueLeft")) + right = _as_number(rule.get("targetValueRight")) + if number is None or left is None or right is None: + return False + left_ok = number >= left if left_op == ">=" else number > left + right_ok = number <= right if right_op == "<=" else number < right + return left_ok and right_ok + return False + + +def _excel_literal(value: Any) -> str: + """Quote a comparison target so Excel treats it as a constant.""" + if isinstance(value, bool): + return "TRUE" if value else "FALSE" + if isinstance(value, (int, float)) and not isinstance(value, bool): + return str(value) + text = str(value).replace('"', '""') + return f'"{text}"' + + +def _header_matches(sheet_header: str, column: str) -> bool: + """Match a rule column to a header, including Pivot ``SUM(col)`` titles.""" + left = sheet_header.strip() + right = column.strip() + if left == right or left.lower() == right.lower(): + return True + return left.endswith(f"({right})") or left.endswith(f"({right.lower()})") + + +def _rule_column_aliases( + column: str, verbose_map: Optional[Mapping[str, Any]] +) -> list[str]: + """Raw and verbose names so rules still match after datasource rename.""" + aliases = [column] + if not verbose_map: + return aliases + verbose = verbose_map.get(column) + if verbose is not None and str(verbose) not in aliases: + aliases.append(str(verbose)) + for raw, label in verbose_map.items(): + if str(label) == column and str(raw) not in aliases: + aliases.append(str(raw)) + return aliases + + +def _column_index( + sheet: Worksheet, + header_row: int, + column: str, + verbose_map: Optional[Mapping[str, Any]] = None, +) -> Optional[int]: + aliases = _rule_column_aliases(column, verbose_map) + for col_idx in range(1, sheet.max_column + 1): + header_label = "" + for row in range(header_row, 0, -1): + value = sheet.cell(row=row, column=col_idx).value + if value not in (None, ""): + header_label = str(value) + break + if header_label and any(_header_matches(header_label, name) for name in aliases): + return col_idx + return None + + +def _data_range(sheet: Worksheet, col_idx: int, header_row: int) -> Optional[str]: + last_row = sheet.max_row + first_row = header_row + 1 + if last_row < first_row: + return None + letter = get_column_letter(col_idx) + return f"{letter}{first_row}:{letter}{last_row}" + + +def _solid_fill(rgb: str) -> PatternFill: + return PatternFill(start_color=rgb, end_color=rgb, fill_type="solid") + + +def _add_formula_rule( + sheet: Worksheet, + cell_range: str, + formula: str, + rgb: str, + *, + text_color: bool, +) -> None: + fill = None if text_color else _solid_fill(rgb) + font = Font(color=rgb) if text_color else None + sheet.conditional_formatting.add( + cell_range, + FormulaRule(formula=[formula], fill=fill, font=font), + ) + + +def _add_cell_is_rule( + sheet: Worksheet, + cell_range: str, + operator: str, + formula: list[str], + rgb: str, + *, + text_color: bool, +) -> None: + fill = None if text_color else _solid_fill(rgb) + font = Font(color=rgb) if text_color else None + sheet.conditional_formatting.add( + cell_range, + CellIsRule(operator=operator, formula=formula, fill=fill, font=font), + ) + + +def _add_data_bar(sheet: Worksheet, cell_range: str, rgb: str) -> None: + sheet.conditional_formatting.add( + cell_range, + DataBarRule( + start_type="min", + end_type="max", + color=rgb, + showValue=True, + minLength=None, + maxLength=None, + ), + ) + + +def _matching_cell_range( + sheet: Worksheet, + col_idx: int, + header_row: int, + rule: dict[str, Any], +) -> Optional[str]: + """Union of cells that match a CELL_BAR comparator, or the full column.""" + operator = rule.get("operator") + if operator in (None, "None", ""): + return _data_range(sheet, col_idx, header_row) + letter = get_column_letter(col_idx) + first_row = header_row + 1 + last_row = sheet.max_row + if last_row < first_row: + return None + runs: list[tuple[int, int]] = [] + run_start: Optional[int] = None + for row in range(first_row, last_row + 1): + matched = _value_matches_rule(sheet.cell(row=row, column=col_idx).value, rule) + if matched and run_start is None: + run_start = row + elif not matched and run_start is not None: + runs.append((run_start, row - 1)) + run_start = None + if run_start is not None: + runs.append((run_start, last_row)) + if not runs: + return None + parts = [ + f"{letter}{start}" if start == end else f"{letter}{start}:{letter}{end}" + for start, end in runs + ] + return " ".join(parts) + + +def _apply_rule( + sheet: Worksheet, + header_row: int, + rule: dict[str, Any], + verbose_map: Optional[Mapping[str, Any]] = None, +) -> None: + column = rule.get("column") + if not isinstance(column, str) or not column: + return + col_idx = _column_index(sheet, header_row, column, verbose_map) + if col_idx is None: + return + cell_range = _data_range(sheet, col_idx, header_row) + if cell_range is None: + return + + rgb = _rule_color(rule) + operator = rule.get("operator") + object_fmt = rule.get("objectFormatting") or "" + text_color = object_fmt == _OBJECT_TEXT + top_left = cell_range.split(":", 1)[0] + + if object_fmt == _OBJECT_CELL_BAR: + bar_range = _matching_cell_range(sheet, col_idx, header_row, rule) + if bar_range: + _add_data_bar(sheet, bar_range, rgb) + return + + if operator in _CELL_IS_OPERATORS: + _add_cell_is_rule( + sheet, + cell_range, + _CELL_IS_OPERATORS[operator], + [_excel_literal(rule.get("targetValue"))], + rgb, + text_color=text_color, + ) + return + + if operator in _RANGE_OPERATORS: + left_op, right_op = _RANGE_OPERATORS[operator] + left = _excel_literal(rule.get("targetValueLeft")) + right = _excel_literal(rule.get("targetValueRight")) + _add_formula_rule( + sheet, + cell_range, + ( + f"AND(NOT(ISBLANK({top_left}))," + f"{top_left}{left_op}{left}," + f"{top_left}{right_op}{right})" + ), + rgb, + text_color=text_color, + ) + return + + if operator in (None, "None", ""): + if text_color: + _add_formula_rule( + sheet, + cell_range, + f"NOT(ISBLANK({top_left}))", + rgb, + text_color=True, + ) + return + min_bound = _as_number(rule.get("minBound")) + max_bound = _as_number(rule.get("maxBound")) + start_type = "num" if min_bound is not None else "min" + end_type = "num" if max_bound is not None else "max" + sheet.conditional_formatting.add( + cell_range, + ColorScaleRule( + start_type=start_type, + start_value=min_bound, + start_color=_SCALE_LOW, + end_type=end_type, + end_value=max_bound, + end_color=rgb, + ), + ) + + +def _is_numeric_header(sheet: Worksheet, col_idx: int, header_row: int) -> bool: + sample_last = min(sheet.max_row, header_row + _NUMERIC_SAMPLE_ROWS) + for row in range(header_row + 1, sample_last + 1): + cell = sheet.cell(row=row, column=col_idx) + if cell.value in (None, ""): + continue + if cell.data_type == "n" or isinstance(cell.value, (int, float)): + return True + return False + return False + + +def _apply_cell_bars(sheet: Worksheet, header_row: int) -> None: + """Attach Excel data bars on numeric columns (Table ``show_cell_bars``).""" + for col_idx in range(1, sheet.max_column + 1): + if not _is_numeric_header(sheet, col_idx, header_row): + continue + cell_range = _data_range(sheet, col_idx, header_row) + if cell_range is None: + continue + _add_data_bar(sheet, cell_range, _DATA_BAR_POSITIVE) + + +def apply_conditional_formatting( + workbook_bytes: bytes, + rules: list[dict[str, Any]], + header_rows: int = 1, + *, + show_cell_bars: bool = False, + verbose_map: Optional[Mapping[str, Any]] = None, +) -> bytes: + """Attach native Excel CF for Explore rules and optional Table cell bars.""" + if not rules and not show_cell_bars: + return workbook_bytes + + workbook: Workbook = load_workbook(io.BytesIO(workbook_bytes)) + sheet = workbook.active + # xlsxwriter omits a full dimension; without this openpyxl can see only + # the header row and skip every highlight. + refresh_sheet_bounds(sheet) + header_row = max(header_rows, 1) + for rule in rules: + if isinstance(rule, dict): + _apply_rule(sheet, header_row, rule, verbose_map) + if show_cell_bars and not rules: + _apply_cell_bars(sheet, header_row) + + output = io.BytesIO() + workbook.save(output) + return output.getvalue() + + +def polish_explore_xlsx( + workbook_bytes: bytes, + df: pd.DataFrame, + form_data: dict[str, Any], + include_index: bool = False, + verbose_map: Optional[Mapping[str, Any]] = None, +) -> bytes: + """Apply Explore number formats and conditional formatting to an XLSX.""" + viz_type = form_data.get("viz_type") + if viz_type not in ("table", "pivot_table_v2"): + return workbook_bytes + + header_rows = getattr(df.columns, "nlevels", 1) + if include_index: + header_rows = max(header_rows, getattr(df.index, "nlevels", 1)) + + skip_display = viz_type == "pivot_table_v2" and form_data.get("showValuesAs") in ( + SHOW_VALUES_AS_PERCENT_MODES + ) + if not skip_display: + headers = [str(column) for column in df.columns] + if viz_type == "table": + styles = styles_from_table_form_data(headers, form_data, verbose_map) + workbook_bytes = apply_column_display( + workbook_bytes, + styles, + header_rows=header_rows, + ordered_headers=headers, + index_columns=( + int(getattr(df.index, "nlevels", 1)) if include_index else 0 + ), + ) + else: + styles = styles_from_pivot_form_data(headers, form_data) + workbook_bytes = apply_column_display( + workbook_bytes, styles, header_rows=header_rows + ) + + rules = form_data.get("conditionalFormatting") or form_data.get( + "conditional_formatting" + ) + if not isinstance(rules, list): + rules = [] + return apply_conditional_formatting( + workbook_bytes, + rules, + header_rows=header_rows, + show_cell_bars=bool(form_data.get("show_cell_bars")), Review Comment: **Suggestion:** When `show_cell_bars` is absent, the frontend defaults it to true, but this expression disables cell bars for older or minimal saved chart configurations. **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Api mismatch` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=715e7abf98af47c6ad627eee60925b55&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=715e7abf98af47c6ad627eee60925b55&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/utils/excel_conditional.py **Line:** 515:515 **Comment:** *Api Mismatch: When `show_cell_bars` is absent, the frontend defaults it to true, but this expression disables cell bars for older or minimal saved chart configurations. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=5454f952aeddae984aee659145cbc55c42ae3a611d64de9d97da6e1118198725&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=5454f952aeddae984aee659145cbc55c42ae3a611d64de9d97da6e1118198725&reaction=dislike'>👎</a> ########## superset/utils/excel_display.py: ########## @@ -0,0 +1,315 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Map Explore d3 number/time formats onto Excel cell formats after a workbook +has been written. + +The export path keeps values numeric (JSON reports stringify them). This module +only changes how Excel *displays* those values so the sheet matches the chart. +""" + +from __future__ import annotations + +import io +import re +from collections.abc import Sequence +from dataclasses import dataclass +from datetime import date, datetime +from typing import Any, Mapping + +from openpyxl.reader.excel import load_workbook +from openpyxl.styles import Alignment + +# Same grammar as ``superset.utils.number_format.D3_FORMAT_RE``. +D3_FORMAT_RE = re.compile( + r"^(?:(.)?([<>=^]))?([+\-( ])?([$#])?(0)?(\d+)?(,)?(?:\.(\d+))?(~)?([a-z%])?$", + re.IGNORECASE, +) +SMART_NUMBER = "SMART_NUMBER" +SMART_NUMBER_SIGNED = "SMART_NUMBER_SIGNED" + +ALLOWED_ALIGNMENTS = frozenset({"left", "center", "right"}) + +# strftime tokens used by Table/Pivot time format controls → Excel format codes. +_STRFTIME_TO_EXCEL: tuple[tuple[str, str], ...] = ( + ("%Y", "yyyy"), + ("%y", "yy"), + ("%m", "mm"), + ("%d", "dd"), + ("%H", "hh"), + ("%I", "hh"), + ("%M", "mm"), + ("%S", "ss"), + ("%p", "AM/PM"), + ("%b", "mmm"), + ("%B", "mmmm"), +) + +_CURRENCY_EXCEL_SYMBOL = { + "USD": "$", + "EUR": "€", + "GBP": "£", + "JPY": "¥", + "CNY": "¥", + "INR": "₹", + "RUB": "₽", + "MXN": "MX$", +} + + +@dataclass(frozen=True) +class ExcelColumnDisplay: + """Display options for one exported column, keyed by header text.""" + + number_format: str | None = None + alignment: str | None = None + + +def d3_number_to_excel( + d3_format: str | None, + currency: Mapping[str, Any] | None = None, +) -> str | None: + """ + Translate a d3-format specifier into an Excel ``numFmt``. + + SMART_NUMBER / SI (``s``) have no Excel equivalent and return ``None`` so + the cell stays General. Unknown specifiers also return ``None``. + """ + if not d3_format or d3_format in {SMART_NUMBER, SMART_NUMBER_SIGNED}: + return _currency_excel_format("#,##0.00", currency) if currency else None + + stripped = d3_format.replace("$", "") + match = D3_FORMAT_RE.fullmatch(stripped) + if not match: + return None + + comma, precision, ntype = match.group(7), match.group(8), (match.group(10) or "f") + ntype = ntype.lower() + if ntype in {"s", "e"}: + return None + + digits = int(precision) if precision is not None else (0 if ntype in {"d", "i"} else 2) + decimals = "" if digits == 0 else "." + ("0" * digits) + grouped = "#,##0" if comma else "0" + body = f"{grouped}{decimals}" + + if ntype == "%": + body = f"{body}%" + + sign = match.group(3) + if sign == "+": + body = f"+{body};-{body}" + elif sign == "(": + body = f"{body};({body})" + + return _currency_excel_format(body, currency) + + +def d3_time_to_excel(d3_time_format: str | None) -> str | None: + """Translate a Python/d3 strftime string into an Excel date format.""" + if not d3_time_format: + return None + excel = d3_time_format + for token, replacement in _STRFTIME_TO_EXCEL: + excel = excel.replace(token, replacement) + if "%" in excel: + return None + return excel or None + + +def _currency_excel_format( + number_body: str, currency: Mapping[str, Any] | None +) -> str: + if not currency: + return number_body + code = str(currency.get("symbol") or "") + if not code or code == "AUTO": + return number_body + symbol = _CURRENCY_EXCEL_SYMBOL.get(code, code) + quoted = f'"{symbol}"' + if str(currency.get("symbolPosition") or "prefix").lower() == "suffix": + return f"{number_body}{quoted}" + return f"{quoted}{number_body}" + + +def _table_header_aliases( + column: str, verbose_map: Mapping[str, Any] | None +) -> list[str]: + """Raw and verbose names so ``column_config`` keys match exported headers.""" + aliases = [column] + if not verbose_map: + return aliases + verbose = verbose_map.get(column) + if verbose is not None and str(verbose) not in aliases: + aliases.append(str(verbose)) + for raw, label in verbose_map.items(): + if str(label) == column and str(raw) not in aliases: + aliases.append(str(raw)) + return aliases + + +def _header_for_column_config( + config_key: str, + header_set: set[str], + verbose_map: Mapping[str, Any] | None, +) -> str | None: + for alias in _table_header_aliases(config_key, verbose_map): + if alias in header_set: + return alias + return None + + +def _parse_horizontal_align(value: Any) -> str | None: + if isinstance(value, str) and value.lower() in ALLOWED_ALIGNMENTS: + return value.lower() + return None + + +def styles_from_table_form_data( + column_headers: list[Any], + form_data: Mapping[str, Any], + verbose_map: Mapping[str, Any] | None = None, +) -> dict[str, ExcelColumnDisplay]: + """Build header → display map from Table ``column_config``.""" + column_config = form_data.get("column_config") or {} + if not isinstance(column_config, dict): + return {} + + styles: dict[str, ExcelColumnDisplay] = {} + header_set = {str(header) for header in column_headers} + for name, config in column_config.items(): + if not isinstance(config, dict): + continue + header = _header_for_column_config(str(name), header_set, verbose_map) + if header is None: + continue + alignment = _parse_horizontal_align(config.get("horizontalAlign")) + currency = config.get("currencyFormat") + currency = currency if isinstance(currency, dict) else None + number_format = d3_number_to_excel( + config.get("d3NumberFormat"), currency + ) or d3_time_to_excel(config.get("d3TimeFormat")) + if number_format or alignment: + styles[header] = ExcelColumnDisplay( + number_format=number_format, alignment=alignment + ) + return styles + + +def styles_from_pivot_form_data( + column_headers: list[Any], + form_data: Mapping[str, Any], +) -> dict[str, ExcelColumnDisplay]: + """ + Apply ``valueFormat`` / per-metric ``columnFormats`` to pivoted headers. + + Flattened export headers are ``" ".join(levels)``, so a metric name is + matched when it is the header or appears as a suffix/prefix token. + """ + value_format = form_data.get("valueFormat") or form_data.get("number_format") + column_formats = form_data.get("columnFormats") or {} + currency = form_data.get("currencyFormat") or form_data.get("currency_format") + currency = currency if isinstance(currency, dict) else None + + styles: dict[str, ExcelColumnDisplay] = {} + for header in column_headers: + text = str(header) + d3 = None + if isinstance(column_formats, dict): + for metric, fmt in column_formats.items(): + if metric and (text == metric or text.endswith(metric) or text.startswith(metric)): + d3 = fmt + break Review Comment: **Suggestion:** Prefix and suffix matching can select a shorter metric before the actual metric, applying the wrong number format when metric names overlap. **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Logic error` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=549c9de8067e47c49d140269041654ec&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=549c9de8067e47c49d140269041654ec&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/utils/excel_display.py **Line:** 233:236 **Comment:** *Logic Error: Prefix and suffix matching can select a shorter metric before the actual metric, applying the wrong number format when metric names overlap. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=2515363d77818bfc33c64ae7d57877e94b9ee3913a5da595a45c32663556ffd4&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=2515363d77818bfc33c64ae7d57877e94b9ee3913a5da595a45c32663556ffd4&reaction=dislike'>👎</a> ########## superset/utils/excel_display.py: ########## @@ -0,0 +1,315 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Map Explore d3 number/time formats onto Excel cell formats after a workbook +has been written. + +The export path keeps values numeric (JSON reports stringify them). This module +only changes how Excel *displays* those values so the sheet matches the chart. +""" + +from __future__ import annotations + +import io +import re +from collections.abc import Sequence +from dataclasses import dataclass +from datetime import date, datetime +from typing import Any, Mapping + +from openpyxl.reader.excel import load_workbook +from openpyxl.styles import Alignment + +# Same grammar as ``superset.utils.number_format.D3_FORMAT_RE``. +D3_FORMAT_RE = re.compile( + r"^(?:(.)?([<>=^]))?([+\-( ])?([$#])?(0)?(\d+)?(,)?(?:\.(\d+))?(~)?([a-z%])?$", + re.IGNORECASE, +) +SMART_NUMBER = "SMART_NUMBER" +SMART_NUMBER_SIGNED = "SMART_NUMBER_SIGNED" + +ALLOWED_ALIGNMENTS = frozenset({"left", "center", "right"}) + +# strftime tokens used by Table/Pivot time format controls → Excel format codes. +_STRFTIME_TO_EXCEL: tuple[tuple[str, str], ...] = ( + ("%Y", "yyyy"), + ("%y", "yy"), + ("%m", "mm"), + ("%d", "dd"), + ("%H", "hh"), + ("%I", "hh"), + ("%M", "mm"), + ("%S", "ss"), + ("%p", "AM/PM"), + ("%b", "mmm"), + ("%B", "mmmm"), +) + +_CURRENCY_EXCEL_SYMBOL = { + "USD": "$", + "EUR": "€", + "GBP": "£", + "JPY": "¥", + "CNY": "¥", + "INR": "₹", + "RUB": "₽", + "MXN": "MX$", +} + + +@dataclass(frozen=True) +class ExcelColumnDisplay: + """Display options for one exported column, keyed by header text.""" + + number_format: str | None = None + alignment: str | None = None + + +def d3_number_to_excel( + d3_format: str | None, + currency: Mapping[str, Any] | None = None, +) -> str | None: + """ + Translate a d3-format specifier into an Excel ``numFmt``. + + SMART_NUMBER / SI (``s``) have no Excel equivalent and return ``None`` so + the cell stays General. Unknown specifiers also return ``None``. + """ + if not d3_format or d3_format in {SMART_NUMBER, SMART_NUMBER_SIGNED}: + return _currency_excel_format("#,##0.00", currency) if currency else None + + stripped = d3_format.replace("$", "") + match = D3_FORMAT_RE.fullmatch(stripped) + if not match: + return None + + comma, precision, ntype = match.group(7), match.group(8), (match.group(10) or "f") + ntype = ntype.lower() + if ntype in {"s", "e"}: + return None + + digits = int(precision) if precision is not None else (0 if ntype in {"d", "i"} else 2) + decimals = "" if digits == 0 else "." + ("0" * digits) + grouped = "#,##0" if comma else "0" + body = f"{grouped}{decimals}" Review Comment: **Suggestion:** Valid d3 `n` and `r` formats are treated as fixed decimal formats, changing grouping and significant-digit display in exported Excel files. **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Type error` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6d5e0196207f40eebb536f6184443c64&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6d5e0196207f40eebb536f6184443c64&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/utils/excel_display.py **Line:** 105:108 **Comment:** *Type Error: Valid d3 `n` and `r` formats are treated as fixed decimal formats, changing grouping and significant-digit display in exported Excel files. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=fbd496b5e98e3b9a9c4ecb62db807727276a97a19db892504531d74e17675ce6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=fbd496b5e98e3b9a9c4ecb62db807727276a97a19db892504531d74e17675ce6&reaction=dislike'>👎</a> ########## superset/utils/excel_display.py: ########## @@ -0,0 +1,315 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Map Explore d3 number/time formats onto Excel cell formats after a workbook +has been written. + +The export path keeps values numeric (JSON reports stringify them). This module +only changes how Excel *displays* those values so the sheet matches the chart. +""" + +from __future__ import annotations + +import io +import re +from collections.abc import Sequence +from dataclasses import dataclass +from datetime import date, datetime +from typing import Any, Mapping + +from openpyxl.reader.excel import load_workbook +from openpyxl.styles import Alignment + +# Same grammar as ``superset.utils.number_format.D3_FORMAT_RE``. +D3_FORMAT_RE = re.compile( + r"^(?:(.)?([<>=^]))?([+\-( ])?([$#])?(0)?(\d+)?(,)?(?:\.(\d+))?(~)?([a-z%])?$", + re.IGNORECASE, +) +SMART_NUMBER = "SMART_NUMBER" +SMART_NUMBER_SIGNED = "SMART_NUMBER_SIGNED" + +ALLOWED_ALIGNMENTS = frozenset({"left", "center", "right"}) + +# strftime tokens used by Table/Pivot time format controls → Excel format codes. +_STRFTIME_TO_EXCEL: tuple[tuple[str, str], ...] = ( + ("%Y", "yyyy"), + ("%y", "yy"), + ("%m", "mm"), + ("%d", "dd"), + ("%H", "hh"), + ("%I", "hh"), + ("%M", "mm"), + ("%S", "ss"), + ("%p", "AM/PM"), + ("%b", "mmm"), + ("%B", "mmmm"), +) + +_CURRENCY_EXCEL_SYMBOL = { + "USD": "$", + "EUR": "€", + "GBP": "£", + "JPY": "¥", + "CNY": "¥", + "INR": "₹", + "RUB": "₽", + "MXN": "MX$", +} + + +@dataclass(frozen=True) +class ExcelColumnDisplay: + """Display options for one exported column, keyed by header text.""" + + number_format: str | None = None + alignment: str | None = None + + +def d3_number_to_excel( + d3_format: str | None, + currency: Mapping[str, Any] | None = None, +) -> str | None: + """ + Translate a d3-format specifier into an Excel ``numFmt``. + + SMART_NUMBER / SI (``s``) have no Excel equivalent and return ``None`` so + the cell stays General. Unknown specifiers also return ``None``. + """ + if not d3_format or d3_format in {SMART_NUMBER, SMART_NUMBER_SIGNED}: + return _currency_excel_format("#,##0.00", currency) if currency else None + + stripped = d3_format.replace("$", "") + match = D3_FORMAT_RE.fullmatch(stripped) + if not match: + return None + + comma, precision, ntype = match.group(7), match.group(8), (match.group(10) or "f") + ntype = ntype.lower() + if ntype in {"s", "e"}: + return None + + digits = int(precision) if precision is not None else (0 if ntype in {"d", "i"} else 2) + decimals = "" if digits == 0 else "." + ("0" * digits) + grouped = "#,##0" if comma else "0" + body = f"{grouped}{decimals}" + + if ntype == "%": + body = f"{body}%" + + sign = match.group(3) + if sign == "+": + body = f"+{body};-{body}" + elif sign == "(": + body = f"{body};({body})" + + return _currency_excel_format(body, currency) + + +def d3_time_to_excel(d3_time_format: str | None) -> str | None: + """Translate a Python/d3 strftime string into an Excel date format.""" + if not d3_time_format: + return None + excel = d3_time_format + for token, replacement in _STRFTIME_TO_EXCEL: + excel = excel.replace(token, replacement) + if "%" in excel: + return None + return excel or None + + +def _currency_excel_format( + number_body: str, currency: Mapping[str, Any] | None +) -> str: + if not currency: + return number_body + code = str(currency.get("symbol") or "") + if not code or code == "AUTO": + return number_body Review Comment: **Suggestion:** `AUTO` currency is discarded instead of being resolved from detected datasource currency, so configured currency columns lose their symbols in Excel exports. **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Api mismatch` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=59345b3dfd3241b1b6e69a8c09e0cf49&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=59345b3dfd3241b1b6e69a8c09e0cf49&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/utils/excel_display.py **Line:** 139:141 **Comment:** *Api Mismatch: `AUTO` currency is discarded instead of being resolved from detected datasource currency, so configured currency columns lose their symbols in Excel exports. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=99b64a974a7b140936d3665eaffbd0983c1e7093faff1906afc472fe5831763b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44719&comment_hash=99b64a974a7b140936d3665eaffbd0983c1e7093faff1906afc472fe5831763b&reaction=dislike'>👎</a> -- 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]
