aminghadersohi commented on code in PR #42609:
URL: https://github.com/apache/superset/pull/42609#discussion_r3688561634
##########
superset/utils/pandas_postprocessing/resample.py:
##########
@@ -46,6 +171,20 @@ def resample(
_("Resample method should be in ") + ", ".join(RESAMPLE_METHOD) +
"."
)
+ tz = df.index.tz
+ df = _pad_to_time_range(
+ df,
+ _coerce_bound(time_range_start, tz),
+ _coerce_bound(time_range_end, tz),
+ )
+ # An empty frame with no time-range anchors has nothing to bin. Returning
+ # early keeps the DatetimeIndex intact; ``resample`` on a zero-length index
+ # would otherwise degrade it to an object Index.
+ if df.empty:
+ return df
+
+ _validate_bucket_count(df.index.min(), df.index.max(), rule)
Review Comment:
The bucket cap runs on **every** `resample()` call, not only when
`fill_time_range`/bounds are supplied. Here `df.index.min()/max()` is the
data's own extent when nothing was padded, so a pre-existing chart that never
opted into this feature — daily over ~30y (10,958 buckets), hourly over 2y
(17,521), or minutely over 10d (14,401) — now raises
`InvalidPostProcessingError` where it rendered before this PR. Verified against
the round-1 base: the same plain call returned a frame there and raises here.
The amplification the cap targets is what this feature introduces, so
consider scoping the check to the padded/bounded path (only when a
`time_range_*` bound was actually applied), and/or making
`MAX_RESAMPLE_BUCKETS` configurable with an `UPDATING.md` note, since it
changes behavior for existing resample users.
--
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]
