ENH: add environment, e.g. "longtable", to Styler.to_latex

[attack68]

checks an item off #41649

closes #37443

adds the longtable template, similar to DataFrame.to_latex.

also allows for a custom environment if \begin{table} should be replaced by \begin{figure} etc.

[jreback]

not sure i love this name, maybe template: and this should be 'longtable', 'table' ?

your comment on position_float is hard to interpret here

[attack68]

environment is the proper LaTeX name for these blocks: LaTeX environments

'longtable' and 'table' are common environments for this but there are others. e.g. see #37443

some arguments are nullified by the use of the 'longtable' environment, such as 'position_float'. can try and rephrase this.

[simonjayhawkins]

maybe should raise ValueError if invalid combinations of parameters are given

[attack68]

there was already validation on position_float so this was non-contentious addition.

[jreback]

can you rebase this

[attack68]

@jreback rebased and updated.

[ivanovmg]

To me it seems like a good extension!
Please find my comments below.

[ivanovmg]

tabular or table?

[attack68]

"tabular" is correct, this is a quirk of the structure of longtable to parametrize the test.

[ivanovmg]

OK, but should it work with table as well?

[attack68]

no because {tabular} is a sub environment of any other environment including {table}, except {longtable}, i.e. we have:

\begin{table}
\begin{tabular}
...           <<-- CSS conversion only done here
\end{tabular}
\end{table}

this tests the inner environment.

[ivanovmg]

In this case I think it is necessary to add one end-to-end test for table with inner tabular.
For now this particular test function test_parse_latex_css_convert_minimal tests only a portion of the output (end of inner environment).

[attack68]

This isn't necessary. There is already test_comprehensive which performs a full output check.

This test is designed to selectively test only the relevant parts of the template(s) which convert css to latex styles, which only occurs in data-cells. Since there are two templates, where the inner environment is defined as either longtable or tabular environment, the test covers both cases.

[ivanovmg]

Right, but in test_comprehensive you do not test environment kwarg.

My point is as follows.
You have a good test for environment='longtable', which compares the full output.
Meanwhile for environment='tabular' there is only a check for the final portion of the output.
How can we ensure that some future changes would not break the upper part of the output when environment='tabular'?

[attack68]

OK, I can see some confusion has arisen here, since environment="tabular" should never be used by a user. It would result in an outer and inner {tabular} environment erroneously. I have used it here as a shortcut to not needing 2 variables. However, I have now amended this test to avoid the confusion.

But with respect to broader testing I think cases are covered through dependencies, for example:

• test_minimal_latex_tabular: tests just the core {tabular} structure without additions.
• test_longtable_minimal: tests just the core {longtable} structure without additions.
• test_comprehensive: tests the core outer {table} and inner {tabular} structure.
• test_longtable_comprehensive: tests the core {longtable} structure with features.
• test_latex_environment: checks {table} is properly substituted for another value in the outer structure.

However, reflecting on your comments, I have removed test_latex_enviroment and have, instead, incorporated it now into test_comprehensive as parameters, which I think you will prefer, and also this increased the power of the test.

[ivanovmg]

See my comments.
For some reason mypy fails.
I guess it is not because of your changes...

[ivanovmg]

OK, but should it work with table as well?

[ivanovmg]

Would it not be clearer if you used an f-string here with environment as a placeholder, while

if environment is None:
    environment = "table"

[attack68]

I could either:

• use an f-string and need to replace every curly brace with a double to avoid iterpretation as a var
• break the dedent into multiple parts with f-string only around {table} to avoid the above
• use replace at end

I subjectively chose the latter, which I felt was clearest - should not be a blocker.

[attack68]

@jreback can this be merged as is? I've addressed almost all comments, with the one subjective suggestion ignored due to preference.

@ivanovmg thanks for review - I think after the iterations the tests are better and clearer.

[jreback]

small comment here. can you list the valid values? (or provide a link to them). can be a followon.

[jreback]

lgtm small comment for followup.