SEPURI-SAI-KRISHNA commented on code in PR #45433:
URL: https://github.com/apache/superset/pull/45433#discussion_r4371372204
##########
superset/utils/csv.py:
##########
@@ -115,6 +130,56 @@ 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):
+ # Every row label is one of its level's distinct values, so a frame
with
+ # nothing to escape is recognised from ``levels`` without a pass over
+ # the rows. The reverse does not hold: ``levels`` keeps values no row
+ # uses any more, which a filter or a slice leaves behind.
+ if any(needs_escaping(level) for level in index.levels):
+ # Only a level carrying a formula label among its row labels is
+ # rebuilt, and that is decided from the row labels rather than from
+ # ``levels``, because the rebuild costs a level its dtype:
+ # ``Index.map`` re-infers it, so a level of integers and nulls
comes
+ # back ``float64``, which writes an integer id as ``5.0`` and
rounds
+ # one past 2**53 to a different number. A stale ``levels`` entry
+ # would pay that for a label the export does not contain, while a
+ # level that does need rebuilding holds a string among its rows,
+ # which keeps the re-inferred dtype ``object``.
+ #
+ # A sibling keeps its own values and codes, and with them its
dtype.
+ # ``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. The row values are read 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(values.map(escape_values))
+ if needs_escaping(values := index.get_level_values(position))
+ else (index.codes[position], level)
+ for position, level in enumerate(index.levels)
+ ]
+ index = pd.MultiIndex(
+ levels=[level for _, level in rebuilt],
+ codes=[level_codes for level_codes, _ in rebuilt],
+ names=index.names,
+ )
+ else:
+ index = escape_axis_level(index)
+ df = df.set_axis(index, axis="index")
Review Comment:
Confirmed, both copy. `pd.options.mode.copy_on_write` is False here, and
neither result shares data with the frame it came from.
Measured on an all-`float64` frame with `index=False` and nothing to escape,
peak extra allocation falls by exactly one copy of the frame:
| data | before | after |
| --- | --- | --- |
| 9.6 MB | 52.69 MB | 43.09 MB |
| 19.2 MB | 82.54 MB | 63.34 MB |
| 38.4 MB | 164.11 MB | 125.70 MB |
Assigned onto the frame instead. `df` is already private, as you say, and the
`rename` copies the index object along with the data, so nothing the caller
still holds is reachable from here. Checked it rather than assuming: the
frame
handed in comes back unchanged in labels, axis names, columns and data
across a
hostile label, a hostile axis name, hostile MultiIndex names, a hostile
column
axis name and the `index=False` path. No mutation separates that, since even
assigning the names in place does not reach the caller, so it stays an
assertion on the existing single-index case rather than a test of its own.
`set_names` takes a list for a flat index and a MultiIndex alike, so both
branches still converge on one assignment.
Census found the same shape once more, at `excel.py:156`, where
`quote_formulas` ends on `rename_axis` after a `rename` that has already
copied. Measured at one extra copy of the frame, 9.60 MB on 9.6 MB of data.
That one predates this branch and is not in the CSV path, so I have left it
alone rather than widen this PR. Happy to send it as its own change.
--
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]
