SEPURI-SAI-KRISHNA commented on code in PR #45407:
URL: https://github.com/apache/superset/pull/45407#discussion_r4371488399
##########
superset/utils/excel.py:
##########
@@ -49,12 +49,30 @@
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
- )
+ """Prefix a string with a quote when it would parse as a formula.
+
+ A string whose first character is ``=`` is stored as a formula cell by the
+ writer, so quoting it is what keeps a formula out of the workbook. Dropping
+ the quote would put one in.
+
+ Leading whitespace is stripped before the prefix is read, so ``" =cmd"`` is
+ quoted like ``"=cmd"``. That part is for consistency across the export
paths
+ rather than to stop the workbook evaluating it: XlsxWriter builds a formula
+ cell only from a string whose first character is ``=``, so a value with
Review Comment:
Good catch, and the sentence was false rather than merely broad. Narrowed it.
Measured through `write_row` on xlsxwriter 3.2.9, `{=cmd()}` is stored as
`<f t="array">cmd()</f>`. The `strings_to_formulas` option does not help: the
array branch in `_write_token_as_string` is not gated on it, so with
`strings_to_formulas: False` a bare `=1+1` becomes a plain string while
`{=cmd()}` is still a formula. It needs both the `{=` prefix and the `}`
suffix, `{1+1}` stays a string, and leading whitespace defeats it like it
defeats `=`. All three guards pass it through unchanged, so it reaches the
sheet from `df_to_excel` and the streaming writer alike.
The docstrings now say only what the change relies on, that whitespace in
front
of the prefix stops the writer recognising a formula at all, and say outright
that the set of unprefixed strings it treats as formulas is wider than
`FORMULA_PREFIXES`, naming the `{=...}` form as not covered here.
The same overstatement was in four more places, in `excel_streaming.py`,
`excel_tests.py`, `excel_streaming_tests.py` and the PR description itself.
All
five are corrected.
Keeping the guard fix out of this PR, since it is a behaviour change and this
one is documentation. Filing it as its own issue and PR, credited to you.
--
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]
