-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
EHN: df.to_latex(escape=True)
also escape index names
#61307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
EHN: df.to_latex(escape=True)
also escape index names
#61307
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
@@ -824,6 +824,33 @@ def test_to_latex_escape_special_chars(self): | |||
) | |||
assert result == expected | |||
|
|||
def test_to_latex_escape_special_chars_in_index_names(self): | |||
special_characters = ["&", "%", "$", "#", "_", "{", "}", "~", "^", "\\"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you start the first line of the test with a reference to the issue:
# https://github.com/pandas-dev/pandas/issues/61309
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
cc @attack68 as the codeowner of styler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
pandas/core/generic.py
Outdated
@@ -3584,6 +3585,7 @@ def _wrap(x, alt_format_): | |||
elif isinstance(header, (list, tuple)): | |||
relabel_index_.append({"labels": header, "axis": "columns"}) | |||
format_index_ = [index_format_] # column_format is overwritten | |||
format_index_names_ = [index_format_] # column_format is overwritten |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to do this here. Currently:
df = pd.DataFrame({'_A': [1], '_B': ['a']}).set_index("_A")
df.columns.name = "_C"
print(df.to_latex(escape=True, header=["_B"]))
# \begin{tabular}{ll}
# \toprule
# _C & _B \\
# \_A & \\
# \midrule
# 1 & a \\
# \bottomrule
# \end{tabular}
whereas I think one would expect \_C
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense. I updated the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also "_B", since it wont render in latex otherwise?
On second thought maybe this is ambiguous, and the code written already handles the alternative and expects a user to input their header override directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code written already handles the alternative and expects a user to input their header override directly
Agreed, if we were to pursue a change here, it should be a separate issue.
Can you add a test for columns.name
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.Stage 3. Part of an multi-stages effort: #57880 (comment)
Part 2 has a issue at #59324
But in the process of implementing, I realized the current implementation of
df.to_latex(escape)
does not go throughstyler.to_latex
but to callstyler.format_index
. This implementation follows the same flow. So maybe part 2 is not really relevant.