codeant-ai-for-open-source[bot] commented on code in PR #42609:
URL: https://github.com/apache/superset/pull/42609#discussion_r3688232698
##########
superset/utils/pandas_postprocessing/resample.py:
##########
@@ -22,12 +23,83 @@
from superset.exceptions import InvalidPostProcessingError
from superset.utils.pandas_postprocessing.utils import RESAMPLE_METHOD
+TimeBound = Union[datetime, str]
-def resample(
+
+def _coerce_bound(
+ value: Optional[TimeBound], tz: Optional[tzinfo]
+) -> Optional[pd.Timestamp]:
+ """
+ Normalize a time range boundary into a ``Timestamp`` comparable with the
index.
+
+ :param value: Boundary as a datetime or a parseable string.
+ :param tz: Timezone of the DataFrame index, if any.
+ :return: Timestamp aligned with the index timezone awareness, or None.
+ :raises InvalidPostProcessingError: If the boundary cannot be parsed.
+ """
+ if value is None:
+ return None
+ try:
+ timestamp = pd.Timestamp(value)
+ except (TypeError, ValueError) as ex:
+ raise InvalidPostProcessingError(
+ _("Invalid time range boundary for resample: %(value)s",
value=value)
+ ) from ex
+
+ if timestamp.tzinfo is None:
+ return timestamp if tz is None else timestamp.tz_localize(tz)
+ # an index and a boundary in different timezones would append into an
+ # object-dtype index that ``resample`` cannot bin
+ return timestamp.tz_localize(None) if tz is None else
timestamp.tz_convert(tz)
+
+
+def _pad_to_time_range(
+ df: pd.DataFrame,
+ time_range_start: Optional[pd.Timestamp],
+ time_range_end: Optional[pd.Timestamp],
+) -> pd.DataFrame:
+ """
+ Add empty rows at the edges of the target period.
+
+ ``DataFrame.resample`` derives its bins from the first and last index
entries,
+ so a series that only covers part of the requested time range is only
filled
+ between its own extremes. Anchoring the index to the boundaries of the
period
+ makes pandas emit buckets for the whole period instead.
+
+ :param df: DataFrame with a DatetimeIndex.
+ :param time_range_start: Inclusive lower boundary of the period.
+ :param time_range_end: Exclusive upper boundary of the period.
+ :return: DataFrame whose index spans the target period.
+ """
+ index = df.index
+ anchors = []
+
+ if time_range_start is not None and (index.empty or time_range_start <
index.min()):
+ anchors.append(time_range_start)
+
+ if time_range_end is not None:
+ # the upper boundary of a Superset time range is exclusive, so anchor
on
+ # the last instant that still belongs to the period
+ last_instant = time_range_end - pd.Timedelta(1, unit="ns")
+ if index.empty or last_instant > index.max():
+ anchors.append(last_instant)
Review Comment:
**Suggestion:** The padding anchors are inserted at the literal query
boundaries, but `df.resample(rule)` uses its own default bucket origin rather
than treating those anchors as the bucket grid. For a range such as 10:30–13:00
with an hourly rule, the result can start at 10:00 or otherwise include buckets
outside the requested interval, so the output does not represent the requested
time range correctly. Align the boundaries or resampling origin to the rule
before padding. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Non-aligned ranges can produce buckets outside requested boundaries.
- ⚠️ Resample charts may show misleading leading or trailing zero buckets.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=864e2a423288418aac36aa4993ff271c&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=864e2a423288418aac36aa4993ff271c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/pandas_postprocessing/resample.py
**Line:** 77:85
**Comment:**
*Incorrect Condition Logic: The padding anchors are inserted at the
literal query boundaries, but `df.resample(rule)` uses its own default bucket
origin rather than treating those anchors as the bucket grid. For a range such
as 10:30–13:00 with an hourly rule, the result can start at 10:00 or otherwise
include buckets outside the requested interval, so the output does not
represent the requested time range correctly. Align the boundaries or
resampling origin to the rule before padding.
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%2F42609&comment_hash=f51620f3eca0b682acca6f1bc4b88c6aeaf34a111b3e9e736860f815ae6dbf6b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42609&comment_hash=f51620f3eca0b682acca6f1bc4b88c6aeaf34a111b3e9e736860f815ae6dbf6b&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]
