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

Enable :widths: option in table directives. #795

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jhcole
Copy link
Contributor

@jhcole jhcole commented Jul 23, 2023

Closes #793

The output from myst-docutils-html5 differs from the results I'm getting from jupyter-book build and I'm still tracking down where the difference are coming from.

There are 4 possible states for the width option of a table directive (not set, "auto", "grid", or a list of integers).

The results from myst-docutils-html5 are as follows:

Option Previous behavior New behavior
Not set (default) no class or colspecs no class or colspecs
auto no class or colspecs no class or colspecs
grid no class or colspecs Add colgroup with column widths set equal to each other.
a list of integers no class or colspecs Add colgroup with column widths set in proportion to the integers given.

The results from jupyter-book build are as follows:

Option Previous behavior New behavior
Not set (default) Add the class "colwidths-auto" to the table and not set any column widths. Set equal proportion widths to each column. This is a regression I hope to fix.
auto Add the class "colwidths-auto" to the table and not set any column widths. Add the class "colwidths-auto" to the table and not set any column widths (no change in behavior).
grid Add the classes "colwidths-auto" and "colwidths-given" to the table, and not set any column widths. Add the class "colwidths-given" to the table, and set equal widths to each column. The documentation claims this option will set widths based on column contents, but it just sets them all equal.
a list of integers Add the classes "colwidths-auto" and "colwidths-given" to the table, and not set any column widths. Add the class "colwidths-given" to the table and set columns widths in proportion to the integers given.

@codecov
Copy link

codecov bot commented Jul 23, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (350c633) 90.26% compared to head (7cf70b4) 90.26%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #795      +/-   ##
==========================================
- Coverage   90.26%   90.26%   -0.01%     
==========================================
  Files          23       23              
  Lines        2970     2969       -1     
==========================================
- Hits         2681     2680       -1     
  Misses        289      289              
Flag Coverage Δ
pytests 90.26% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
myst_parser/mdit_to_docutils/base.py 93.51% <ø> (-0.01%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chrisjsewell
Copy link
Member

chrisjsewell commented Jul 23, 2023

The output from myst-docutils-html5 differs from the results I'm getting from jupyter-book build and I'm still tracking down where the difference are coming from.

To note, this shouldn't have anything to do with jupyter-book, since jupyter-book is just a thin wrapper on top of sphinx+myst-parser, that does not change its behaviour
If there is a difference, it would either be between docutils vs. sphinx behaviour, or a change in behaviour between versions of docutils/sphinx.

This is the code for the table directive: https://github.com/live-clones/docutils/blob/6de53a0de5415174d58e775110d89e13dd76fc83/docutils/docutils/parsers/rst/directives/tables.py#L134

This is where it used in the HTML conversion: https://github.com/live-clones/docutils/blob/6de53a0de5415174d58e775110d89e13dd76fc83/docutils/docutils/writers/_html_base.py#L686

@goroderickgo
Copy link

Hi, I saw the linked issue and was wondering if there was a roadmap to have this PR get merged into a release? Thanks!

@jhcole
Copy link
Contributor Author

jhcole commented Mar 9, 2024

Hi, I saw the linked issue and was wondering if there was a roadmap to have this PR get merged into a release? Thanks!

Sorry, I haven't had time to finish this. It will likely be a few months before I have time to focus on it again. You're welcome to take it over.

@berlin2123
Copy link

It works In my sphinx, If one just only change the code in /usr/local/lib/python3.<version>/dist-packages/myst_parser/mdit_to_docutils/base.py as this PR.

i.e., delete or comment the line 1352:

        #table["classes"] += ["colwidths-auto"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Column widths set in a table directive are ignored.
4 participants