sadpandajoe commented on code in PR #45433:
URL: https://github.com/apache/superset/pull/45433#discussion_r4365066147
##########
superset/utils/csv.py:
##########
@@ -115,6 +130,48 @@ def escape_values(v: Any) -> Union[str, Any]:
if isinstance(value, str):
df.at[label, name] = escape_value(value)
+ # Row labels and axis names are written out whenever the caller asks for
the
+ # index, which both export paths do once post-processing has replaced the
+ # range index: pivot_table_v2 promotes the row dimension's warehouse values
+ # into index labels and the dimension name into ``index.name``. Escaping
+ # happens after the cell loop so the labels ``.at`` addresses stay the ones
+ # the frame was handed over with. Every level of a MultiIndex is covered;
+ # the axis names are a separate attribute, hence the ``rename_axis``. This
+ # mirrors the four places ``superset.utils.excel.quote_formulas`` covers
+ # for the xlsx writer.
+ index = df.index
+ if isinstance(index, pd.MultiIndex):
+ # ``levels`` carries each level's distinct values, so an index with
+ # nothing to escape is recognised without a pass over every row.
+ if any(needs_escaping(level) for level in index.levels):
+ # Only a level carrying a formula label is rebuilt. A sibling keeps
+ # its own values and codes, and with them its dtype: read back row
+ # by row it would lose a missing value to ``float64``, which writes
+ # an integer id as ``5.0`` and rounds one past 2**53 to a different
+ # number. ``escape_value`` prepends a quote, so two distinct labels
+ # can escape to the same text, and ``factorize`` folds those onto a
+ # single code, which the level values of a ``MultiIndex`` have to
+ # be unique for. It reads the level's row values rather than
+ # ``levels``, which is sorted rather than in row order, so reusing
+ # the original codes against escaped ``levels`` would put each
+ # label on the wrong row as soon as a hostile one did not sort
+ # first.
+ rebuilt = [
+
pd.factorize(index.get_level_values(position).map(escape_values))
+ if needs_escaping(level)
Review Comment:
`needs_escaping(level)` reads `index.levels`, which keeps values no row uses
any more (after a filter/slice), so a formula label that isn't in any exported
row still triggers the rebuild of that level. The rebuild goes through the row
values, so a mixed level such as `["=a", 9007199254740993, None]` sliced to its
last two rows exports `9007199254740992.0` where master writes
`9007199254740993`, with nothing to escape in the output. Should the decision
be made from the values actually present in the rows (e.g.
`index.get_level_values(position)`) rather than from `levels`?
--
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]
