rusackas commented on code in PR #43396:
URL: https://github.com/apache/superset/pull/43396#discussion_r3832253855
##########
tests/unit_tests/pandas_postprocessing/test_prophet.py:
##########
@@ -180,6 +180,22 @@ def test_prophet_incorrect_periods():
)
+def test_prophet_periods_exceeding_max_raises():
+ """
+ ``periods`` comes from the unvalidated post-processing options dict; the
+ schema-declared upper bound (``MAX_PROPHET_PERIODS``, default 10000) is
+ documentation-only unless enforced at the point ``periods`` is consumed,
+ since every forecast period adds a future row per series.
+ """
+ with pytest.raises(InvalidPostProcessingError, match="must not exceed"):
+ prophet(
+ df=prophet_df,
+ time_grain="P1M",
+ periods=10001,
+ confidence_interval=0.8,
Review Comment:
Good catch, fixed. The over-limit value now comes from the configured max
plus one instead of a hardcoded 10001.
##########
superset/utils/excel.py:
##########
@@ -47,20 +47,28 @@
FORMULA_PREFIXES = {"=", "+", "-", "@"}
+def _quote_formula(value: Any) -> Any:
+ """Prefix a string with a quote when it would parse as a formula."""
+ return (
+ f"'{value}"
+ if isinstance(value, str) and len(value) and value[0] in
FORMULA_PREFIXES
+ else value
+ )
+
+
def quote_formulas(df: pd.DataFrame) -> pd.DataFrame:
"""
Make sure to quote any formulas for security reasons.
"""
for col in df.select_dtypes(include="object").columns:
- df[col] = df[col].apply(
- lambda x: (
- f"'{x}"
- if isinstance(x, str) and len(x) and x[0] in FORMULA_PREFIXES
- else x
- )
- )
-
- return df
+ df[col] = df[col].apply(_quote_formula)
+
+ # Column headers and index labels are written to the sheet as well, and
+ # pivot exports promote data values into both (a hostile warehouse string
+ # can become a header or row label), so quote them like the CSV writer
+ # quotes its headers. ``rename`` applies the mapper to every level of a
+ # MultiIndex.
+ return df.rename(columns=_quote_formula, index=_quote_formula)
Review Comment:
Good catch, fixed. Added rename_axis for index/column names alongside the
existing rename for labels.
##########
superset/utils/pandas_postprocessing/histogram.py:
##########
@@ -45,6 +54,14 @@ def histogram(
and each column corresponds to a histogram bin. The values are
the counts in each bin.
""" # noqa: E501
+ if not isinstance(bins, int) or not 1 <= bins <= MAX_HISTOGRAM_BINS:
Review Comment:
Good catch, fixed. bool is now excluded explicitly before the int/range
check.
##########
superset/utils/pandas_postprocessing/resample.py:
##########
@@ -46,6 +52,27 @@ def resample(
_("Resample method should be in ") + ", ".join(RESAMPLE_METHOD) +
"."
)
+ if len(df):
+ try:
+ step = pd.Timedelta(pd.tseries.frequencies.to_offset(rule))
+ except ValueError:
+ # Non-fixed frequencies (month, quarter, year) have no fixed
+ # Timedelta; their projected row count is bounded by the span in
+ # days and needs no cap. Invalid rules fail in ``df.resample``.
+ step = None
+ if step is not None and step.value > 0:
+ span = df.index.max() - df.index.min()
+ projected_rows = span.value // step.value + 1
+ if projected_rows > MAX_RESAMPLE_ROWS:
Review Comment:
Good catch, fixed. Added a margin to the projected-row estimate to cover the
bin-alignment case.
##########
superset/utils/pandas_postprocessing/histogram.py:
##########
@@ -45,6 +54,14 @@ def histogram(
and each column corresponds to a histogram bin. The values are
the counts in each bin.
""" # noqa: E501
+ if not isinstance(bins, int) or not 1 <= bins <= MAX_HISTOGRAM_BINS:
Review Comment:
Fixed the real gap on this line (bool passing as int) in the sibling CodeAnt
thread on the same line. Not taking the np.integer widening here since bins
only ever arrives as a JSON-decoded plain int from the options dict, never a
numpy scalar.
--
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]