bito-code-review[bot] commented on code in PR #42609:
URL: https://github.com/apache/superset/pull/42609#discussion_r3971626520
##########
superset/utils/pandas_postprocessing/resample.py:
##########
@@ -28,20 +30,184 @@
# rows from a single request.
MAX_RESAMPLE_ROWS = 1_000_000
+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)
+
+ if not anchors:
+ return df
+
+ # `copy` detaches the empty slice from the index engine of `df`, which
would
+ # otherwise refuse to reindex whenever `df` holds duplicate timestamps
+ padding = df.iloc[:0].copy().reindex(pd.DatetimeIndex(anchors,
name=index.name))
+ return pd.concat([df, padding]).sort_index(kind="stable")
+
+
+def _period_freq_for_offset(offset: Any) -> str:
+ """
+ Map a DatetimeIndex/resample offset to a Period frequency string.
+
+ Resample uses anchors like ``MS`` / ``QE`` / ``YE``, but
``Timestamp.to_period``
+ only accepts the Period forms ``M`` / ``Q`` / ``Y`` (and similarly for
week).
+ """
+ # ``QS-JAN``, ``W-SUN``, ``YE-DEC`` → base token before the first hyphen
+ base = offset.freqstr.split("-", 1)[0]
+ return {
+ "MS": "M",
+ "ME": "M",
+ "QS": "Q",
+ "QE": "Q",
+ "YS": "Y",
+ "YE": "Y",
+ "AS": "Y",
+ "A": "Y",
+ }.get(base, offset.freqstr)
Review Comment:
<!-- Bito Reply -->
The fix for the `2MS`/`2QE` frequency mapping issue has been implemented by
stripping the leading numeric multiplier from the base token before mapping it
to the corresponding Period frequency. This ensures that multiplied calendar
rules are correctly resolved instead of falling back to the day-count
estimation, which previously caused overestimation of monthly bins and
potential rejection of valid queries. The CI tests are currently running to
verify these changes.
--
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]
