Skip to content
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

Wrap tabular environments in adjustbox to resize to page width #1937

Merged
merged 9 commits into from
Sep 28, 2022

Conversation

odow
Copy link
Collaborator

@odow odow commented Sep 15, 2022

This most commonly affects tables produced by DatFrames.jl, and resizes them to fit the page if they have many columns that would otherwise run off the right-hand edge of the page.

Closes #1930

This is what it looks like now:

image

I added a {table} environment with \centering, but I can remove if people object.

@odow
Copy link
Collaborator Author

odow commented Sep 15, 2022

Linkcheck failure seems like a network issue. It isn't related to this PR.

@odow odow added the Format: LaTeX Related to the LaTeX / PDF output label Sep 15, 2022
@mortenpi
Copy link
Member

There is still a table in the showcase that manages to overflow.. but that is due to the cells themselves overflowing I guess, which is harder to fight.

image

Could we add one or two examples with tables to the showcase for this?

Also from the showcase, big equations can be problematic too:

image

Do you think adjustboxing them could also work to avoid this?

@odow
Copy link
Collaborator Author

odow commented Sep 18, 2022

I don't know if LaTeX has a way of automatically wrapping text in a table without manually specifying the column widths.

And the equation thing is similarly hard.

@odow
Copy link
Collaborator Author

odow commented Sep 18, 2022

Math issue is #1470, which was fixed for HTML but not for LaTeX.

@mortenpi
Copy link
Member

I am happy to merge this as is too, and leave the equations etc. for the future. If so, this could just use a CHANGELOG note.

This most commonly affects tables produced by DatFrames.jl, and
resizes them to fit the page if they have many columns that would
otherwise run off the right-hand edge of the page.
@odow
Copy link
Collaborator Author

odow commented Sep 20, 2022

Added.

p.s. I'm just about to head to the airport, so I'll only be online sporadically until mid-next week.

@mortenpi
Copy link
Member

Do you think we should also put an adjustbox around the usual tables (as opposed to ones from show methods)? I.e.

function latex(io::Context, node::Node, table::MarkdownAST.Table)
rows = MarkdownAST.tablerows(node)
# latex(io, node.children)
_println(io, "\n\\begin{table}[h]")
_print(io, "\n\\begin{tabulary}{\\linewidth}")
_println(io, "{|", uppercase(join(spec_to_align.(table.spec), '|')), "|}")
for (i, row) in enumerate(rows)
i === 1 && _println(io, "\\hline")
for (j, cell) in enumerate(row.children)
j === 1 || _print(io, " & ")
latex(io, cell.children)
end
_println(io, " \\\\")
_println(io, "\\hline")
end
_println(io, "\\end{tabulary}\n")
_println(io, "\\end{table}\n")
end

@odow
Copy link
Collaborator Author

odow commented Sep 27, 2022

The normal markdown tables already use the {tabulary} environment with the linewidth option, so I think we're okay without?

The problematic tables are really DataFrames because it implements show(::IO, ::MIME"text/latex", ::DataFrame) that produces a {tabular} environment (since it can't use external latex packages).

@mortenpi
Copy link
Member

They can still overflow if they're big enough though (you can look at the at-eval generated table I added to the showcase). However, I'm also happy to leave it for a future improvement.

@odow
Copy link
Collaborator Author

odow commented Sep 28, 2022

I don't really understand why the @eval doesn't get resized, but the @example does?

image

It's another second-order thing. We're heading in the right direction, and getting everything perfect every time is a really hard job.

@mortenpi
Copy link
Member

It's a different latex method that gets called (we print the Markdown[AST].Table directly, and don't rely on the show method), so it doesn't get wrapped in adjustbox right now.

function latex(io::Context, node::Node, table::MarkdownAST.Table)
rows = MarkdownAST.tablerows(node)
# latex(io, node.children)
_println(io, "\n\\begin{table}[h]")
_print(io, "\n\\begin{tabulary}{\\linewidth}")
_println(io, "{|", uppercase(join(spec_to_align.(table.spec), '|')), "|}")
for (i, row) in enumerate(rows)
i === 1 && _println(io, "\\hline")
for (j, cell) in enumerate(row.children)
j === 1 || _print(io, " & ")
latex(io, cell.children)
end
_println(io, " \\\\")
_println(io, "\\hline")
end
_println(io, "\\end{tabulary}\n")
_println(io, "\\end{table}\n")
end

But as I said, I'm more than happy to merge this as is. As you said, mostly it's DataFrames etc. that cause problems, which the current PR already takes care of.

@odow
Copy link
Collaborator Author

odow commented Sep 28, 2022

But as I said, I'm more than happy to merge this as is. As you said, mostly it's DataFrames etc. that cause problems, which the current PR already takes care of.

👍 I'm keen to build the JuMP docs once these PRs are all in to see how much of a difference it makes.

CHANGELOG.md Outdated Show resolved Hide resolved
@mortenpi mortenpi merged commit bea473a into JuliaDocs:master Sep 28, 2022
@odow odow deleted the od/fix-table-output branch September 28, 2022 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Format: LaTeX Related to the LaTeX / PDF output Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overfull elements in LaTeXWriter
2 participants