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

LaTeX: Don't mix booktabs with zebra stripes! #10997

Open
mgeier opened this issue Nov 27, 2022 · 9 comments
Open

LaTeX: Don't mix booktabs with zebra stripes! #10997

mgeier opened this issue Nov 27, 2022 · 9 comments

Comments

@mgeier
Copy link
Contributor

mgeier commented Nov 27, 2022

Describe the bug

I like the new booktabs feature!

I'm not a fan of the zebra stripes, but I guess they are still better than the boring old tables.

But mixing the two is a no-go! That's absolutely ugly!

The idea of the booktabs style is to remove distracting lines, so it is very much counter productive to add big fat background colors to that.

How to Reproduce

Build docs containing a table with the LaTeX builder.

Environment Information

master

Sphinx extensions

No response

Additional context

No response

@jfbu
Copy link
Contributor

jfbu commented Nov 27, 2022

Please provide screenshots demonstrating the ugliness. Zebra-striped rows make row separators not needed. The booktabs has a relatively minimal impact in this context to delimit the header of the table and the final table row.

@mgeier
Copy link
Contributor Author

mgeier commented Nov 29, 2022

Sure, let's look at this example from docutils: https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#grid-tables

latex_table_style = []

image

latex_table_style = ['booktabs']

image

latex_table_style = ['booktabs', 'colorrows']

image

latex_table_style = ['colorrows']

image

latex_table_style = ['colorrows', 'borderless']

image

@jfbu
Copy link
Contributor

jfbu commented Nov 29, 2022

Well, yes, that is indeed the problem with merged cells in grid tables. Perhaps the documentation should warn more about that. Are they really that commonly used? (tremendous effort went into Sphinx LaTeX about 4 or 5 years ago circa to support them at 1.6.1 of May 16, 2017, now that I check CHANGES, so in a way I wish the answer was yes, but realistically I doubt the answer is yes).

As you can see above, merged cells involving a single row are handled fine but as soon as two or more rows are involved, background colouring is turned off. (this is deliberately, as for TeXnical reasons it would be extremely difficult to achieve colouring, assuming to start with some method could be agreed upon to choose which colour is actually to be obeyed...).

Now, I think it would be great if people commented on this issue and helped us decide whether we must maintain zebra-striped tables as default. The decision was taken on the premise that the vast majority of Sphinx users do not use complex grid tables with merged cells.

edit: for documents having only a few complex grid tables it is always possible to style them especillay with some rst-class directive added before the table.

Each table can override the global style via :class: option, or .. rst-class:: for no-directive tables (cf. Tables). Currently recognized classes are booktabs, borderless, standard, colorrows, nocolorrows. The latter two can be combined with any of the first three. The standard class produces tables with both horizontal and vertical lines (as has been the default so far with Sphinx).

@ferdnyc
Copy link
Contributor

ferdnyc commented Dec 16, 2022

@jfbu:

As you can see above, merged cells involving a single row are handled fine

That isn't actually apparent from the examples @mgeier posted, but nevertheless it does appear to be true:

Screenshot from 2022-12-16 00-14-58

And is consistent with this note in the docs, though I admit I misread it at first and thought otherwise:

Multi-row merged cells, whether single column or multi-column currently ignore any set column, row, or cell colour.

Back to @jfbu:

but as soon as two or more rows are involved, background colouring is turned off. (this is deliberately, as for TeXnical reasons it would be extremely difficult to achieve colouring, assuming to start with some method could be agreed upon to choose which colour is actually to be obeyed...).

...I definitely get why multi-row merged cells would interfere with row coloring, but (assuming they're only a single column wide), why would they ignore column colors as well? There shouldn't (in theory) be any conflict there.

It'd be nice if colorrows could automatically disable itself completely if any multi-row merged cells are detected, since as @mgeier points out the results are... not great. (Whether or not it's in combination with booktabs.) But I could certainly accept an explanation that implementing such a feature would require too much effort to be worth it.

@jfbu
Copy link
Contributor

jfbu commented Dec 16, 2022

Thanks @ferdnyc for suggestions, and added screenshots.

...I definitely get why multi-row merged cells would interfere with row coloring, but (assuming they're only a single column wide), why would they ignore column colors as well? There shouldn't (in theory) be any conflict there.

I agree with you, logic would let us expect multirow single column should obey a set column color as multicolumn single row obey a row color. But LaTeX tables are drawn row-wise. In case of a multirow single-column cell, the contents are in fact injected in top cell in a manner spilling over cells which are underneath and those must be kept blank. In particular they should not inject color as they are drawn second, and this color would end up on top of already printed "ink".

EDIT: I forgot to explain that when Sphinx injects the multirow contents it does not know but only estimates the exact heights of the respective rows involved in the merged cell. So a color panel created by the Sphinx latex code using the dimensions of the box it uses to typeset th contents would not extend to the precise location of the last cell floor, it would end earlier, or perhaps further down.

There is a note in Sphinx latex code:

% MEMO at 5.3.0: to allow a multirow cell in a single column to react to
% \columncolor correctly, it seems only way is that the contents
% are inserted by bottom cell (this is mentioned in multirow.sty doc, too).
% Sphinx could at Python level "move" the contents to that cell. But the

I have cut after "but the" as it is followed by ten lines about why this is challenging technically, and the explanation is itself too challenging LaTeXnically ;-)

EDIT: I found a another related comment

% MEMO: we could check status of \CT@cell@color here, but unfortunately we
% can't know the exact height which will be covered by the cells in total
% (it may be more than our \box\z@ dimensions). We could use an \fcolorbox
% wrapper on \box\z@ but this will not extend precisely to the bottom rule.
%
% Only solution if we want to obey a raw \cellcolor, or a \columncolor, seems
% to delay unboxing the gathered contents as part of the bottom row with
% a suitable vertical adjustment...

Beware some only partial solutions you may find if searching on a LaTeX forum for coloring multirow cells, for example color-multirow-in-a-table on tex.sx. The accepted answer wouldn't work for us, as it is incompatible with cells containing paragraphs (btw, Sphinx does not use multirow package because of those limitations). This answer also relies on mark-up inserting the merged contents at the level of the bottom cell, and it is explained in the omitted part of my quote abvoe why this is complicated. The tex.sx answer is for manual mark-up and only works in simple cases of table rows all of the same height. Another problem with this is that it can create artefacts in some PDF viewers, because the background coloring will in fact be done with at least two pieces which may not join exactly and thus cause anti-aliasing issues.

The other answer at color-multirow-in-a-table on tex.sx proposes to use the nicematrix LaTeX package. This is quite recent package which subordinates the rendering of tables to the TikZ/pgf graphics engine. The latter pdf drivers allow to set up absolute marks on the page (which is not available in standard LaTeX) and using a multi-step process can thus a posteriori put things in suitable locations once those locations are known. Unfortunately I am quite sure the package will break the variety of tables Sphinx must support. And it means a complete rewrite of Sphinx latex internals... PRs are welcome of course. Another (even more recent I think) package is tabularray but last time I checked it didn't yet fill the bill of requirements for Sphinx tables.

It'd be nice if colorrows could automatically disable itself completely if any multi-row merged cells are detected, since as @mgeier points out the results are... not great. (Whether or not it's in combination with booktabs.)

Yes, automatic solutions are always to be preferred to bad output. It looks definitely feasible because this can be handled at the level of the Sphinx LaTeX writer, which is Python to produce the LaTeX mark-up. Deactivating colorrows due to multirow merged cells should probably mean also deactivate booktabs else only column separators will remain and merged cells are then visually hard to disambiguate from neighboring cells.

But I could certainly accept an explanation that implementing such a feature would require too much effort to be worth it.

It does not look like too much effort ;-) I will look into it. I guess there should be some new configuration value, probably a Boolean.

Currently, as explained in the docs:

Each table can override the global style via :class: option, or .. rst-class:: for no-directive tables (cf. Tables). Currently recognized classes are booktabs, borderless, standard, colorrows, nocolorrows. The latter two can be combined with any of the first three. The standard class produces tables with both horizontal and vertical lines (as has been the default so far with Sphinx).

@mgeier
Copy link
Contributor Author

mgeier commented Dec 16, 2022

I should have taken a different example!
I simply took the first table I found in the docs, it just happened to be one with colspans and rowspans.

The discussion about the problems that zebra stripes currently have with those colspans and rowspans is interesting, but really isn't the problem I wanted to point out.

The problem is that "booktabs" and "zebra stripes" should not be mixed.

I propose to use ['booktabs'] as a default.

Users can then choose ['colorrows', 'borderless'] if they want zebra stripes.

The options ['booktabs', 'colorrows'] and ['colorrows'] mix lines and zebra stripes. They look very bad and should not be recommended.

@jfbu
Copy link
Contributor

jfbu commented Dec 29, 2022

Hi all, about

The problem is that "booktabs" and "zebra stripes" should not be mixed.

I still do not understand why not. Waiting for others to emit opinion.

About

It'd be nice if colorrows could automatically disable itself completely if any multi-row merged cells are detected, since as @mgeier points out the results are... not great. (Whether or not it's in combination with booktabs.)

Yes, automatic solutions are always to be preferred to bad output. It looks definitely feasible because this can be handled at the level of the Sphinx LaTeX writer, which is Python to produce the LaTeX mark-up. Deactivating colorrows due to multirow merged cells should probably mean also deactivate booktabs else only column separators will remain and merged cells are then visually hard to disambiguate from neighboring cells.

But I could certainly accept an explanation that implementing such a feature would require too much effort to be worth it.

It does not look like too much effort ;-) I will look into it. I guess there should be some new configuration value, probably a Boolean.

I looked at this more closely and it is not easy and does look like it requires effort. A main problem is that the presence of multirow-multicolumn cell is detected only while traversing the table at a time when some decisions have already been taken regarding the output mark-up (which however is still kept into provisory storage and could possibly be altered after but not easily)

def visit_entry(self, node: Element) -> None:
if self.table.col > 0:
self.body.append('&')
self.table.add_cell(node.get('morerows', 0) + 1, node.get('morecols', 0) + 1)
cell = self.table.cell()

Deciding at this traversal time (from the 'morerows' and 'morecols' node attributes set by the DocUtils table parser) to set some signal that the table should use the "standard" legacy style is possible but one would have to a posteriori modify some things, in particular if a prior multicolumn single-row cell had been rendered, as this depends on the "colsep" which may be empty or a "|". Currently this "colsep" is set globally at start of table rendering prior to visiting the rows.

Random thoughts related to these difficulties :

  • DocUtils own LaTeX writer raises a NotImplementedError() when encountereding a multirow-multicolumn cell (from a grid table). Sphinx LaTeX writer supports it but it is expected their occurrences in Sphinx projects are rare. Grid tables are a pain to "paint" and I don't think they are used widely (despite some dedicated Emacs mode), and are the only way to achieve multirow-multicolumn merged cells. They will then be the result of dedicated effort and the user who can then add .. rst-class:: standard at little additional effort.
  • Single row multicolumn cells are handled fine by Sphinx LaTeX in booktabs+zebrastripes context.
  • Single column multirow cells drop the background color (which could then only be the column color (which can only be set from an explicit .. tabularcolumns directive prior to table in source) for the TeXnical difficulties already alluded to.
  • It ia always possible to set latex_table_style = [] in conf.py to maintain legacy behaviour globally.

I understand the original aim of @mgeier was not here to discuss multirow-multicolumn merged cells and that I have devoted much words and paragraphs about it, as it appears to me as being the (only) problem. Because, as I said, I simply do not understand the

The problem is that "booktabs" and "zebra stripes" should not be mixed.

opinion. I still do not understand why.

"Booktabs" ('booktabs') basically means removing all vertical lines, and zebra stripes ('colorrows') is related to providing background colors to rows. Why they should not get mixed is a mystery to me.

@mgeier
Copy link
Contributor Author

mgeier commented Dec 29, 2022

The problem is that "booktabs" and "zebra stripes" should not be mixed.

opinion. I still do not understand why.

There is nothing to understand, it's just bad style.

If you want to try to rationalize it, you could apply the famous quote by Antoine de Saint-Exupéry:

Il semble que la perfection soit atteinte non quand il n'y a plus rien à ajouter, mais quand il n'y a plus rien à retrancher.

https://fr.wikiquote.org/wiki/Terre_des_hommes#Chapitre_III_:_L.27Avion

perfection is finally attained not when there is no longer anything to add, but when there is no longer anything to take away

https://en.wikiquote.org/wiki/Antoine_de_Saint_Exupéry

You can remove the booktabs lines, and you can remove the zebra stripes, in both cases the result is better.

@jfbu
Copy link
Contributor

jfbu commented Dec 29, 2022

@mgeier nice metaphor. Well, we diverge on this I thus get to experience the thrill of feeling a revolutionary... which may end up in somber mood if an avalanche of complaints arrives as Sphinx 6.0.0 is has now been released... we'll see.

Update: I built the numpy-ref.pdf and numpy-user.pdf from https://github.com/numpy/numpy at 1.24.1 and using current 6.x Sphinx tip (223bb31) and I see nothing wrong about booktabs+zebrastripes rendering there. Here is an example table

Capture d’écran 2023-01-05 à 00 49 39

@AA-Turner AA-Turner added this to the some future version milestone Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants