codeant-ai-for-open-source[bot] commented on code in PR #45979:
URL: https://github.com/apache/superset/pull/45979#discussion_r4456594845
##########
superset/charts/client_processing.py:
##########
@@ -1397,6 +1398,267 @@ def apply_pivot_number_formats(
return df.T if metrics_on_rows else df
+def time_comparison_shifts(form_data: dict[str, Any]) -> list[Any]:
+ """The saved time shifts, as ``ensureIsArray`` reads them."""
+ shifts = form_data.get("time_compare")
+ if shifts is None:
+ return []
+ if isinstance(shifts, (list, tuple)):
+ return list(shifts)
+ return [shifts]
+
+
+def is_using_time_comparison(form_data: dict[str, Any]) -> bool:
+ """
+ Whether the Table plugins derive time-comparison columns.
+
+ Mirrors ``isUsingTimeComparison`` in
+ ``plugin-chart-table/src/transformProps.ts``, which the AG Grid plugin
+ repeats verbatim. Both conditions after the shift are read straight off the
+ form data: ``transformProps`` does not infer ``query_mode`` the way
+ ``buildQuery`` does, and ``comparison_type``'s ``values`` default lives in
+ the control rather than in the chart code, so a chart that saved neither
+ derives nothing in the browser either.
+ """
+ return bool(
+ time_comparison_shifts(form_data)
+ and form_data.get("query_mode") == "aggregate"
+ and form_data.get("comparison_type") == "values"
+ )
+
+
+def comparison_suffix(form_data: dict[str, Any]) -> Optional[str]:
+ """
+ The offset whose columns the derivation reads, or ``None`` for no shift.
+
+ Mirrors ``timeOffsets[0]``: the shifts naming a period come first in the
+ order they were saved, a ``custom`` shift contributes its start date and an
+ ``inherit`` shift the literal ``inherit``, and only the first entry is
read.
+ """
+ shifts = time_comparison_shifts(form_data)
+ offsets: list[Any] = [
+ shift for shift in shifts if shift not in ("custom", "inherit")
+ ]
+ if "custom" in shifts:
+ offsets.append(form_data.get("start_date_offset"))
+ if "inherit" in shifts:
+ offsets.append("inherit")
+ if not offsets:
+ return None
+ # The offset is interpolated into a template string before
+ # ``String.prototype.includes`` ever sees it, so a ``custom`` shift saved
+ # without a start date is compared as the text ``undefined`` rather than
+ # suppressing the derivation. It matches no column, every comparison value
+ # reads as zero, and that is what the browser shows.
+ return "undefined" if offsets[0] is None else str(offsets[0])
+
+
+def comparison_value(value: Any) -> Any:
+ """
+ A comparison operand with the browser's ``|| 0`` applied.
+
+ Every read in ``processComparisonDataRecords`` is ``|| 0``, so a null, a
+ missing column and a NaN are all zero before any branch inspects the
+ operand. ``bool(float("nan"))`` is ``True`` in Python while ``NaN`` is
+ falsy in JavaScript, which is why the coercion cannot be left to ``or``.
+ """
+ return 0 if value is None or pd.isna(value) else value
+
+
+def percent_difference(original: Any, comparison: Any) -> float:
+ """
+ ``calculateDifferences``' percent difference, which is a ratio not a
percent.
+
+ A zero operand short-circuits to a whole step in one direction rather than
+ dividing, and the denominator is an absolute value, so a positive metric
+ against a negative comparison grows the ratio instead of flipping its sign.
+ """
+ if not original and not comparison:
+ return 0
+ if not original or not comparison:
+ return 1 if original else -1
+ return (original - comparison) / abs(comparison)
+
+
+def holds_only_numbers(series: pd.Series) -> bool:
+ """
+ Whether every value is a number or null, as ``isNumeric`` decides it.
+
+ ``transformProps`` treats a column as a metric only when ``typeof`` says
+ each of its values is a number, nulls apart, so a metric returning strings
+ is presented as a plain column instead. ``is_numeric_dtype`` answers a
+ different question and disagrees in both directions: it is true of a
+ boolean column, which ``typeof`` calls a boolean, and false of an all-null
+ object column, which has no value to disqualify it.
+ """
+ for value in series:
+ if value is None or pd.isna(value):
Review Comment:
**Suggestion:** For a list-valued metric such as an array aggregate,
`pd.isna(value)` returns an array, so this condition raises instead of leaving
the metric as a plain column.
**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=fa5ea41bb9354c568e85bb4e7d74a92b&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=fa5ea41bb9354c568e85bb4e7d74a92b&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:** 1495:1495
**Comment:**
*Type Error: For a list-valued metric such as an array aggregate,
`pd.isna(value)` returns an array, so this condition raises instead of leaving
the metric as a plain column.
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%2F45979&comment_hash=76a403cc2bac507bf3b80a78a9bc3816e3c49ac65181593ba86545b358fa068a&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45979&comment_hash=76a403cc2bac507bf3b80a78a9bc3816e3c49ac65181593ba86545b358fa068a&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]
