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

Add support for booktabs-style tables to LaTeX builder #6666

Closed
wants to merge 4 commits into from

Conversation

sephalon
Copy link

@sephalon sephalon commented Aug 21, 2019

Subject: Add support for booktabs-style tables to LaTeX builder

Feature or Bugfix

  • Feature

Purpose

Render tables without vertical rules and horizontal rules of varying
thickness (with additional space above and below) using the booktabs
package.

Feature is not polished yet (see below), but should be enough to keep the discussion started.

TODO

  • Support column spans in headers (via writing \cmidrule commands)
  • Allow selective enabling/disabling of booktabs-style tables via a custom option (like :latex-booktabs:)
  • longtable test fails, need to have a look into that

@jfbu
Copy link
Contributor

jfbu commented Aug 22, 2019

+1, reasonable. The loading of LaTeX package booktabs itself should propbably be made conditional on the Python configuration latex_booktabs you are adding. I can't help immediately but will have some time in the coming weeks.

@tk0miya
Copy link
Member

tk0miya commented Oct 19, 2019

Does anyone know other (well-used) table style? If anything exists, latex_table_style might be better slightly for the name of configuration to me.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for your idea! But I feel some refactoring is needed for this. And it seems testing are failed. Could you check it please? I'll take a look later if I have time.

In addition, new testcase is needed for this option. Please add them later after discussion.

sphinx/writers/latex.py Outdated Show resolved Hide resolved
sphinx/templates/latex/longtable.tex_t Outdated Show resolved Hide resolved
@lcnittl
Copy link

lcnittl commented Sep 10, 2020

Not very sure if this is somehow useful, but it seems that docutils has a booktabs option implemented

@sephalon
Copy link
Author

sephalon commented Jun 8, 2021

Sorry, I got sidetracked by other projects… I will rebase this and have a look at your comments.

@tk0miya, what is the status of #7417? It seems to go into a similar direction, but has been closed recently without being merged. Is there a specific reason for that?

sphinx/writers/latex.py Outdated Show resolved Hide resolved
@jfbu
Copy link
Contributor

jfbu commented Aug 6, 2022

I am sorry about very long delay. But the world of LaTeX tables is a terrible jungle mess. I am worried about setting a precedent if we provide a LaTeX package specific adaptation, what if it turns incompatible with using some other package which adds colours to rows etc... There are some other things in the code base which are tied to using default columns lines and need adjustments. Sphinx has a sophisticated architecture with no equivalent in the world of LaTeX table packages to handle grid tables with merged cells. Ideally, the safest would not to worry about external LaTeX packages and provide all necessary customization hooks internally. But this probably means maintenance becomes impossible because requires to much knowledge of LaTeX and TeX macros and primitives.

Anyway I will try to focus my available Sphinx time on this and possibly push to this branch. (hopefully, no promises made..)

@jfbu
Copy link
Contributor

jfbu commented Aug 6, 2022

Not very sure if this is somehow useful, but it seems that docutils has a booktabs option implemented

Indeed. It is actually a value we can pass as one of a comma separated class values to the table_style option of [latex writers]. This raises question whether we should support a similar latex_table_style. Notice that I tested passing borderless and it removes all borders in PDF output using Docutils rst2latex.py but their documentation says

borderless

    No borders around the table.

ah well, html output also removes everything. Anyway I wanted to check if the Docutils class values were additive or mutually exclusive, but here combining booktabs and borderless is not very illustrative as borderless removes all borders anyhow. Anyway, let's worry about this later.

Render tables without vertical rules and horizontal rules of varying
thickness (with additional space above and below) using the booktabs
package.

This is a rebase of original commit which was on master branch prior to
2.1 release.  In this rebase the \RequirePackage{booktabs} has been
removed from sphinx.sty, as it has to be conditional on latex_booktabs
(or whatever its name will be) configuration option, and will be
repositioned in a subsequent commit.
@jfbu
Copy link
Contributor

jfbu commented Aug 6, 2022

I am in the process of rebasing this on 5.x branch, hopefully for 5.2.0 (as it is a feature addition I think it is suitable for 5.2.0 if we maintain exact backwards compatibility).

@jfbu jfbu changed the base branch from master to 5.x August 6, 2022 17:36
@jfbu
Copy link
Contributor

jfbu commented Aug 6, 2022

well, mypy chokes on the colspec in the LaTeX writer Python code probably in relation to this comment and I fill fix this later (perhaps tomorrow).

@jfbu
Copy link
Contributor

jfbu commented Aug 6, 2022

@tk0miya I rebase don 5.x but I hesitate if I should keep this PR for merge to master. Indeed I expect to be replacing \hline mark-up in tex output by \sphinxtoprule, \sphinxmidrule, \sphinxbottomrule defaulting to \hline without the booktabs activation. This means changing the output mark-up in tex file. I have also changed output mark-up recently at 5.1.0 for implementing rounded boxes for code blocks but I am not so sure I can do this on 5.x liberally.

@jfbu
Copy link
Contributor

jfbu commented Aug 6, 2022

I don't know what causes the linkcheck test failures I have seen recently often: https://github.com/sephalon/sphinx/runs/7706483793?check_suite_focus=true#step:8:2088. This time at Python 3.10 but on a nother recent occasion it was Python 3.7. And after merge no failure was reported.

cf https://github.com/sphinx-doc/sphinx/runs/7618138712?check_suite_focus=true for the Python 3.7 instance on commit 5090486 which after merge f72a25c tested fine... (the linkcheck failures being without relation to the actual merged patches).

@jfbu
Copy link
Contributor

jfbu commented Aug 7, 2022

I have reimplemented the whole thing. Apart from removing | from the column specifications and related collaterals, the table mark-up is the same whether the latex_use_booktabs option is True or False: the \sphinxtoprule etc... will expand to the booktabs \toprule only if the option was True.

@jfbu jfbu requested a review from tk0miya August 7, 2022 20:13
@jfbu
Copy link
Contributor

jfbu commented Aug 7, 2022

Ready for review. (but I forgot to add a CHANGES entry)

TBH I think this option will start making sense only when "zebra-style" colouring for tables will also be available (which however may be tricky for complex tables with merged cells, but we can perhaps simply warn).
(relates: #6671).

@jfbu jfbu added this to the 5.2.0 milestone Aug 7, 2022
@jfbu jfbu removed the request for review from tk0miya August 8, 2022 06:53
@jfbu
Copy link
Contributor

jfbu commented Aug 8, 2022

I am closing this as it has been transfered to #10759 for further development.

@jfbu jfbu closed this Aug 8, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
builder:latex type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants