codeant-ai-for-open-source[bot] commented on code in PR #45817:
URL: https://github.com/apache/superset/pull/45817#discussion_r4401013197


##########
superset/charts/client_processing.py:
##########
@@ -1222,6 +1223,87 @@ def merge_currency_formats(
     return currency_formats
 
 
+def apply_export_number_formats(
+    df: pd.DataFrame,
+    coltypes: list[GenericDataType],
+    form_data: dict[str, Any],
+    datasource: Optional[Union["BaseDatasource", "Query"]] = None,
+) -> pd.DataFrame:
+    """Format numeric columns as display text for a chart-data export.
+
+    Each numeric column (by ``coltypes``, aligned with ``df.columns``) is
+    resolved to its configured d3 format, currency and small-number format the
+    same way the live Table plugin does -- ``column_config`` overrides first,
+    then the datasource's saved per-metric formats -- and handed to
+    ``format_column``. A numeric column with nothing configured takes the
+    fixed-point fallback so its cells are never serialized as raw floats.
+    Non-numeric columns, nulls, NaN and infinity are left to the existing
+    helpers and pass through unchanged. The passed frame is mutated in place 
and
+    returned; the helper runs no query and performs no I/O.
+    """
+    saved_formats, verbose_map = get_datasource_column_formats(datasource)
+    saved_currency_formats, _ = get_datasource_currency_formats(datasource)
+    column_config = form_data.get("column_config") or {}
+
+    def label_of(name: str) -> str:
+        """Return the column label as it appears in ``df`` 
(verbose-renamed)."""
+        return name if name in df.columns else verbose_map.get(name, name)
+
+    # Index the per-column overrides by the label present in ``df`` so the
+    # lookups line up with the (possibly verbose-renamed) frame columns.
+    number_format_by_label = {
+        label_of(name): fmt for name, fmt in saved_formats.items()
+    }
+    currency_by_label = {
+        label_of(name): currency for name, currency in 
saved_currency_formats.items()
+    }
+    config_by_label = {label_of(name): config for name, config in 
column_config.items()}
+
+    numeric_columns = {
+        column
+        for column, coltype in zip(df.columns, coltypes, strict=True)
+        if coltype == GenericDataType.NUMERIC
+    }
+
+    for column in df.columns:
+        if column not in numeric_columns:
+            continue
+
+        config = config_by_label.get(column) or {}
+        configured_currency = config.get("currencyFormat") or {}
+        # The small-number formatter is built from this object whether or not a
+        # symbol survived in it, so an absent one has to stay distinguishable
+        # from a cleared one.
+        small_currency = config.get("currencyFormat")
+        number_format = config.get("d3NumberFormat") or 
number_format_by_label.get(
+            column
+        )
+        currency = (
+            configured_currency
+            if configured_currency.get("symbol")
+            else currency_by_label.get(column) or {}
+        )
+        # A cleared Select leaves a blank string behind, which the browser
+        # treats as unset.
+        small_number_format = (config.get("d3SmallNumberFormat") or 
"").strip() or None
+
+        if number_format or currency.get("symbol") or small_number_format:
+            format_column(
+                df,
+                column,
+                number_format,
+                currency,
+                small_d3_format=small_number_format,
+                small_currency=small_currency,
+            )

Review Comment:
   **Suggestion:** `AUTO` is passed without detected currency or row currency 
context, so it remains unresolved and exports amounts without the configured 
currency symbol.
   
   **Assessment:** ๐ŸŸ  `Major` ยท ๐Ÿ” `Occurrence: Sometimes` ยท ๐Ÿท๏ธ `Api mismatch`
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
 [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1c5a6b3b78c1455499b06cfe478be236&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1c5a6b3b78c1455499b06cfe478be236&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/charts/client_processing.py
   **Line:** 1291:1298
   **Comment:**
        *Api Mismatch: `AUTO` is passed without detected currency or row 
currency context, so it remains unresolved and exports amounts without the 
configured currency symbol.
   
   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%2F45817&comment_hash=19f145b3f7a29c0a1fcd23e6f7be4e86851a26af2f4bd9c8c15bf282cf1e1180&reaction=like'>๐Ÿ‘</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45817&comment_hash=19f145b3f7a29c0a1fcd23e6f7be4e86851a26af2f4bd9c8c15bf282cf1e1180&reaction=dislike'>๐Ÿ‘Ž</a>



##########
superset/charts/client_processing.py:
##########
@@ -1222,6 +1223,87 @@ def merge_currency_formats(
     return currency_formats
 
 
+def apply_export_number_formats(
+    df: pd.DataFrame,
+    coltypes: list[GenericDataType],
+    form_data: dict[str, Any],
+    datasource: Optional[Union["BaseDatasource", "Query"]] = None,
+) -> pd.DataFrame:
+    """Format numeric columns as display text for a chart-data export.
+
+    Each numeric column (by ``coltypes``, aligned with ``df.columns``) is
+    resolved to its configured d3 format, currency and small-number format the
+    same way the live Table plugin does -- ``column_config`` overrides first,
+    then the datasource's saved per-metric formats -- and handed to
+    ``format_column``. A numeric column with nothing configured takes the
+    fixed-point fallback so its cells are never serialized as raw floats.
+    Non-numeric columns, nulls, NaN and infinity are left to the existing
+    helpers and pass through unchanged. The passed frame is mutated in place 
and
+    returned; the helper runs no query and performs no I/O.
+    """
+    saved_formats, verbose_map = get_datasource_column_formats(datasource)
+    saved_currency_formats, _ = get_datasource_currency_formats(datasource)
+    column_config = form_data.get("column_config") or {}
+
+    def label_of(name: str) -> str:
+        """Return the column label as it appears in ``df`` 
(verbose-renamed)."""
+        return name if name in df.columns else verbose_map.get(name, name)
+
+    # Index the per-column overrides by the label present in ``df`` so the
+    # lookups line up with the (possibly verbose-renamed) frame columns.
+    number_format_by_label = {
+        label_of(name): fmt for name, fmt in saved_formats.items()
+    }
+    currency_by_label = {
+        label_of(name): currency for name, currency in 
saved_currency_formats.items()
+    }
+    config_by_label = {label_of(name): config for name, config in 
column_config.items()}
+
+    numeric_columns = {
+        column
+        for column, coltype in zip(df.columns, coltypes, strict=True)
+        if coltype == GenericDataType.NUMERIC
+    }
+
+    for column in df.columns:
+        if column not in numeric_columns:
+            continue
+
+        config = config_by_label.get(column) or {}
+        configured_currency = config.get("currencyFormat") or {}
+        # The small-number formatter is built from this object whether or not a
+        # symbol survived in it, so an absent one has to stay distinguishable
+        # from a cleared one.
+        small_currency = config.get("currencyFormat")
+        number_format = config.get("d3NumberFormat") or 
number_format_by_label.get(
+            column
+        )
+        currency = (
+            configured_currency
+            if configured_currency.get("symbol")
+            else currency_by_label.get(column) or {}
+        )
+        # A cleared Select leaves a blank string behind, which the browser
+        # treats as unset.
+        small_number_format = (config.get("d3SmallNumberFormat") or 
"").strip() or None
+
+        if number_format or currency.get("symbol") or small_number_format:

Review Comment:
   **Suggestion:** Percent metrics default to `PERCENT_3_POINT` in the Table 
chart, but this condition sends them to the unconfigured fallback and exports 
fractions such as `0.3`.
   
   **Assessment:** ๐ŸŸ  `Major` ยท ๐Ÿ” `Occurrence: Sometimes` ยท ๐Ÿท๏ธ `Logic error`
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
 [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bcfc5a247c324da0b2d92a0633435cba&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bcfc5a247c324da0b2d92a0633435cba&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/charts/client_processing.py
   **Line:** 1290:1290
   **Comment:**
        *Logic Error: Percent metrics default to `PERCENT_3_POINT` in the Table 
chart, but this condition sends them to the unconfigured fallback and exports 
fractions such as `0.3`.
   
   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%2F45817&comment_hash=967e1aee628d289d3024dd2a0b36ad70aec93187a25b9be1d8473ffd87cc3dd3&reaction=like'>๐Ÿ‘</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45817&comment_hash=967e1aee628d289d3024dd2a0b36ad70aec93187a25b9be1d8473ffd87cc3dd3&reaction=dislike'>๐Ÿ‘Ž</a>



##########
superset/common/query_context_processor.py:
##########
@@ -644,6 +646,14 @@ def get_data(
             if verbose_map:
                 df.columns = [verbose_map.get(column, column) for column in 
columns]
 
+            if is_feature_enabled("EXPORT_FORMATTED_NUMBERS"):
+                apply_export_number_formats(
+                    df,
+                    coltypes,
+                    self._query_context.form_data or {},
+                    self._qc_datasource,
+                )

Review Comment:
   **Suggestion:** On XLSX exports, `excel.apply_column_types` converts 
formatted strings like `3.50` back to numeric `3.5`, so the exported cell loses 
its configured decimal places.
   
   **Assessment:** ๐ŸŸ  `Major` ยท ๐Ÿ” `Occurrence: Sometimes` ยท ๐Ÿท๏ธ `Type error`
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
 [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7f799c714a9d454d80861e63944f344e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7f799c714a9d454d80861e63944f344e&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/common/query_context_processor.py
   **Line:** 650:655
   **Comment:**
        *Type Error: On XLSX exports, `excel.apply_column_types` converts 
formatted strings like `3.50` back to numeric `3.5`, so the exported cell loses 
its configured decimal places.
   
   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%2F45817&comment_hash=c976527da06b616933d6106ff8454f06cba3eb4bb9d4b6e129eabe082e00954a&reaction=like'>๐Ÿ‘</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45817&comment_hash=c976527da06b616933d6106ff8454f06cba3eb4bb9d4b6e129eabe082e00954a&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]

Reply via email to