rusackas commented on code in PR #42609:
URL: https://github.com/apache/superset/pull/42609#discussion_r3972633479
##########
superset/utils/pandas_postprocessing/resample.py:
##########
@@ -28,20 +30,157 @@
# 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 _estimate_projected_rows(start: pd.Timestamp, end: pd.Timestamp, rule:
str) -> int:
+ """
+ Estimate how many bins ``resample(rule)`` would produce between two bounds.
+
+ Fixed-duration rules use Timedelta arithmetic plus a +2 alignment margin
+ (pandas may snap bins outside the observed span). Calendar frequencies
+ (month, quarter, year, …) have no fixed Timedelta; those are estimated via
+ period arithmetic so the DoS cap still applies to them.
+ """
+ if end < start:
+ return 0
+ offset = to_offset(rule)
+ try:
+ nanos = offset.nanos
+ except ValueError:
+ # Non-fixed frequencies: convert bounds to periods of ``rule`` and
+ # subtract. This avoids materializing a multi-million-row DatetimeIndex
+ # just to decide whether to reject the request.
+ try:
+ return int(end.to_period(rule) - start.to_period(rule)) + 1
+ except (TypeError, ValueError):
+ return len(pd.date_range(start=start, end=end, freq=rule))
+ if nanos <= 0:
+ return 0
Review Comment:
Confirmed fixed and re-verified independently — ran _estimate_projected_rows
against real pandas 2.3.3 for MS/QE/YE and their multiples, all within 0-1 of
the true bucket count from pd.date_range, none undercounting. The Period branch
is genuinely reached now, no date_range materialization on this path.
##########
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:
Confirmed fixed and re-verified independently. Ran the actual code against
real pandas: 2MS/3MS/2QS/2QE/2YE all now estimate within 0-1 of pd.date_range's
true count (previously off by ~60x per your report). Also checked it never
undercounts, which is what actually matters for a safety cap.
--
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]
