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

[BUG] Putting a Rule in a Table cell interacts weirdly with table width and other column widths/ratios #2266

Closed
gimbo opened this issue May 9, 2022 · 4 comments

Comments

@gimbo
Copy link
Contributor

gimbo commented May 9, 2022

Overview

There seems to be something weird happening when I put a Rule inside a Table cell. Perhaps that's a weird thing to be doing in itself, but OTOH it feels like it should be possible, particularly as Table.grid is proposed as a general layout tool for Renderables.

The behaviour I'd expect would be that adding a Rule to a table cell doesn't modify the table or column widths from what they'd be without that Rule being present — but that's not what's happening now, AFACIS.

(Of course layout is a Hard Problem, so this might be one of those edge cases where you end up playing whack-a-mole...)

Anyway, here's the weirdness; there are three main things, but I guess they're related so I report them together:

  1. A Rule in a table cell seems to effectively force expand=True on the table — even if it's explicitly False.

    • Expectation: while a rule might contribute to a column's / table's width, it wouldn't necessarily force the table to expand to full width.
  2. If a table has expand=True, a rule in some column causes that column to take up as much width as possible.

    • Expectation: column widths would be unchanged by addition of rule. E.g. if columns were symmetric without Rule, they'd be symmetric with it.
  3. (This one's real weird.) If a table of two columns has ratio=1 for one column, and if the non-ratio column contains a Rule, that completely squashes the first table column to a single character width (and we also get the expand=True forcing described above).

    • Expectation: column widths would be unchanged by addition of rule.
    • Caveat: semantics of ratio don't seem documented in cases where some columns have ratio set and some don't (?), but it seems like something sensible happens, at least when there are no Rules involved, i.e.:
      • If expand=True, columns with no ratio take up as little space as possible. Seems optimal. ✅
      • If expand=False, the ratios make no difference. Makes sense I guess.

Example

I set myself up a playground for exploring this. The main thing it enables is the exploration of this space of options, in the context of a table with two columns and three rows:

  • Whether Table.expand is True or False.
  • Whether column 1 has ratio=1 or no ratio set.
  • Whether the second row in column 2 contains a Rule or some text.

Those were the main things; I also explore less fully:

  • Whether column 1 has min_width=10 or no min_width set.
  • Whether column 2 has min_width=10 or no min_width set.
Show/hide example source code
from dataclasses import dataclass
from enum import Flag, auto

from rich import box
from rich.console import Console
from rich.rule import Rule
from rich.table import Table


class F(Flag):
    """Various flags controlling how table is set up."""

    # Main flags we're interested in exploring
    T_EXPAND = auto()  # Does table have expand=True or False?
    C1_RATIO = auto()  # Does column 1 have ratio=1 set?
    C2_RULED = auto()  # Does column 2 contain a cell with a Rule in it?

    # Secondary flags which we're interested in, but less so...
    C1_MIN = auto()  # Does column 1 have min_width=10 set?
    C2_MIN = auto()  # Does column 2 have min_width=10 set?


class Flags(set):
    @property
    def table_kwargs(self):
        return {'expand': F.T_EXPAND in self}

    @property
    def col_1_kwargs(self):
        kwargs = {}
        if F.C1_MIN in self:
            kwargs['min_width'] = 10
        if F.C1_RATIO in self:
            kwargs['ratio'] = 1
        return kwargs

    @property
    def col_2_kwargs(self):
        kwargs = {}
        if F.C2_MIN in self:
            kwargs['min_width'] = 10
        return kwargs

    @property
    def col_2_rule_cell(self):
        return Rule() if F.C2_RULED in self else 'XXXXX'

    @property
    def summary(self):
        return (
            f"  [yellow]Table kwargs[/yellow] : {self.table_kwargs}\n"
            + f"  [yellow]Col 1[/yellow]        : {self.col_1_kwargs}\n"
            + f"  [yellow]Col 2[/yellow]        : {self.col_2_kwargs}\n"
            + f"  [yellow]Col 2 ruled[/yellow]  : {F.C2_RULED in self}"
        )


@dataclass
class RuleDemoTable:

    flags: Flags
    note: str

    DEFAULT_TABLE_KWARGS = {'box': box.ASCII, 'show_header': False}

    def table(self) -> Table:
        """Compute Table renderable."""
        table = Table(**(self.DEFAULT_TABLE_KWARGS | self.flags.table_kwargs))
        table.add_column(**self.flags.col_1_kwargs)
        table.add_column(**self.flags.col_2_kwargs)
        table.add_row('COL 1', 'COL 2')
        table.add_row('COL 1', self.flags.col_2_rule_cell)
        table.add_row('COL 1', 'COL 2')
        return table


def t(flags, note):
    return RuleDemoTable(Flags(flags), note)


TABLES = [
    # Unexpanded tables; rule forces expansion, ratio ignored
    t(set(), 'No flags set, expand=False; compact table'),
    t({F.C2_RULED}, 'Rule forces table expansion'),
    t({F.C1_RATIO}, 'Compact table; ratio ignored :man_shrugging:'),
    t({F.C1_RATIO, F.C2_RULED}, 'Rule forces expansion; ratio ignored'),
    # Expanded tables... rule claims all the space
    t({F.T_EXPAND}, 'Expanded, no ratio; symmetric columns; looks good'),
    t({F.T_EXPAND, F.C2_RULED}, 'Expanded, no ratio; rule makes col 2 claim width'),
    t({F.T_EXPAND, F.C1_RATIO}, 'Expanded, ratio set: col 1 claims width'),
    # This is the really bad one!
    t({F.T_EXPAND, F.C1_RATIO, F.C2_RULED}, 'Expanded, ratio set: rule squashes col 1'),
    # This one's just a bit odd/surprising (nothing to do with rules)
    t({F.T_EXPAND, F.C1_MIN}, 'Expanded, col 1 min_width, no rule; col 2 too wide?'),
    t({F.T_EXPAND, F.C1_MIN, F.C2_RULED}, 'This actually seems OK, even with rule! :-)'),
    # Setting C2 min...
    t({F.T_EXPAND, F.C2_MIN, F.C2_RULED}, 'This seems sensible too... :-)'),
]


def main():
    import platform
    import sys
    from importlib.metadata import version

    console = Console(record=True, width=80)
    console.print()
    for n, table in enumerate(TABLES, start=1):
        console.print(f'[cyan]Table {n}')
        console.print(table.table())
        console.print(f'[white]{table.note}')
        console.print(table.flags.summary)
        console.print()
        console.print()
    console.print('Platform: ' + platform.platform())
    console.print('Python:')
    console.print('  ' + sys.version.replace('\n', ''))
    console.print('rich: ' + version('rich'))
    console.save_text('rule_issue_demo_output.txt', clear=False, styles=False)
    console.save_svg('rule_issue_demo_output.svg', title='Rule issue demo output')


if __name__ == '__main__':
    main()

Output of example

Of course the example code uses rich, so... 😄

Show/hide SVG output

rule_issue_demo_output

Show/hide plain text output
Table 1
+---------------+
| COL 1 | COL 2 |
| COL 1 | XXXXX |
| COL 1 | COL 2 |
+---------------+
No flags set, expand=False; compact table
  Table kwargs : {'expand': False}
  Col 1        : {}
  Col 2        : {}
  Col 2 ruled  : False


Table 2
+------------------------------------------------------------------------------+
| COL 1 | COL 2                                                                |
| COL 1 | ──────────────────────────────────────────────────────────────────── |
| COL 1 | COL 2                                                                |
+------------------------------------------------------------------------------+
Rule forces table expansion
  Table kwargs : {'expand': False}
  Col 1        : {}
  Col 2        : {}
  Col 2 ruled  : True


Table 3
+---------------+
| COL 1 | COL 2 |
| COL 1 | XXXXX |
| COL 1 | COL 2 |
+---------------+
Compact table; ratio ignored 🤷‍♂️
  Table kwargs : {'expand': False}
  Col 1        : {'ratio': 1}
  Col 2        : {}
  Col 2 ruled  : False


Table 4
+------------------------------------------------------------------------------+
| COL 1 | COL 2                                                                |
| COL 1 | ──────────────────────────────────────────────────────────────────── |
| COL 1 | COL 2                                                                |
+------------------------------------------------------------------------------+
Rule forces expansion; ratio ignored
  Table kwargs : {'expand': False}
  Col 1        : {'ratio': 1}
  Col 2        : {}
  Col 2 ruled  : True


Table 5
+------------------------------------------------------------------------------+
| COL 1                                 | COL 2                                |
| COL 1                                 | XXXXX                                |
| COL 1                                 | COL 2                                |
+------------------------------------------------------------------------------+
Expanded, no ratio; symmetric columns; looks good
  Table kwargs : {'expand': True}
  Col 1        : {}
  Col 2        : {}
  Col 2 ruled  : False


Table 6
+------------------------------------------------------------------------------+
| COL 1 | COL 2                                                                |
| COL 1 | ──────────────────────────────────────────────────────────────────── |
| COL 1 | COL 2                                                                |
+------------------------------------------------------------------------------+
Expanded, no ratio; rule makes col 2 claim width
  Table kwargs : {'expand': True}
  Col 1        : {}
  Col 2        : {}
  Col 2 ruled  : True


Table 7
+------------------------------------------------------------------------------+
| COL 1                                                                | COL 2 |
| COL 1                                                                | XXXXX |
| COL 1                                                                | COL 2 |
+------------------------------------------------------------------------------+
Expanded, ratio set: col 1 claims width
  Table kwargs : {'expand': True}
  Col 1        : {'ratio': 1}
  Col 2        : {}
  Col 2 ruled  : False


Table 8
+------------------------------------------------------------------------------+
|| COL 2                                                                    |
| 1 |                                                                          |
|| ──────────────────────────────────────────────────────────────────────── |
| 1 |                                                                          |
|| COL 2                                                                    |
| 1 |                                                                          |
+------------------------------------------------------------------------------+
Expanded, ratio set: rule squashes col 1
  Table kwargs : {'expand': True}
  Col 1        : {'ratio': 1}
  Col 2        : {}
  Col 2 ruled  : True


Table 9
+------------------------------------------------------------------------------+
| COL 1                                           | COL 2                      |
| COL 1                                           | XXXXX                      |
| COL 1                                           | COL 2                      |
+------------------------------------------------------------------------------+
Expanded, col 1 min_width, no rule; col 2 too wide?
  Table kwargs : {'expand': True}
  Col 1        : {'min_width': 10}
  Col 2        : {}
  Col 2 ruled  : False


Table 10
+------------------------------------------------------------------------------+
| COL 1      | COL 2                                                           |
| COL 1      | ─────────────────────────────────────────────────────────────── |
| COL 1      | COL 2                                                           |
+------------------------------------------------------------------------------+
This actually seems OK, even with rule! :-)
  Table kwargs : {'expand': True}
  Col 1        : {'min_width': 10}
  Col 2        : {}
  Col 2 ruled  : True


Table 11
+------------------------------------------------------------------------------+
| COL 1 | COL 2                                                                |
| COL 1 | ──────────────────────────────────────────────────────────────────── |
| COL 1 | COL 2                                                                |
+------------------------------------------------------------------------------+
This seems sensible too... :-)
  Table kwargs : {'expand': True}
  Col 1        : {}
  Col 2        : {'min_width': 10}
  Col 2 ruled  : True


Platform: macOS-12.3.1-arm64-arm-64bit
Python:
  3.9.9 (main, Feb 14 2022, 23:43:16) [Clang 13.0.0 (clang-1300.0.29.30)]
rich: 12.4.1

Discussion

Tables which don't surprise me:

  • Table 1 — not much to say, not much going on here. 😄

  • Table 3 — makes sense that ratio is inapplicable if the table isn't expanded or has no min_width.

  • Table 5 — also not much to say; nicely symmetric.

  • Table 7 — this shows ratio=1 causing column 1 to take up as much space as it can, which makes sense to me if no ratio is set on column 2.

  • Table 10 — this demonstrates that the Rule doesn't squash a min_width column, which is a relief.

  • Table 11 — similar to table 10, except here the min_width is on column 2, not column 1. OK.

Surprises:

  • Table 2 — as soon as a Rule appears, it's as if Table.expand=True even though we set it False. I'd expect this to look like table 1 but with a small rule in place of the XXXXX.

  • Table 4 — similar comment, but here we also have ratio=1 so if the Rule being there did expand the table, I'd expect column 1 to take up the width, not column 2.

  • Table 6 — I'd expect this have the same column widths as table 5.

  • Table 8 — this is the big one — here we've set ratio=1 on the first column, and now the rule completely squashes it. This is the example I hit in my actual use case... a real WTF moment. 😄

  • Table 9 — minor surprise here, and nothing to do with a Rule — just wouldn't expect min_width to cause asymmetric columns here. Maybe a separate issue report is in order here though...?

Again, maybe all of this is basically expected behaviour; the ratio semantics seem loosely defined, and perhaps Rule is simply not intended to be in a table. But if the latter is true, it reduces the utility of Table.grid for layout.

None of this is world-ending, but I thought it was worth exploring at least...

Finally, thank you 🎉 for such a useful and well-constructed tool; it really is great. I hadn't heard of rich or textualize until your interview on Talk Python To Me — really glad I did! 🙏🏻

Platform info

Show/hide platform info

I'm on macOS 12.3.1 (21E258) on a MacBook Pro (13-inch, M1, 2020). I'm using iTerm2 build 3.4.15. And as the example code shows:

Platform: macOS-12.3.1-arm64-arm-64bit
Python:
  3.9.9 (main, Feb 14 2022, 23:43:16) [Clang 13.0.0 (clang-1300.0.29.30)]
rich: 12.4.1

Diagnostics

$ python -m rich.diagnose
╭───────────────────────── <class 'rich.console.Console'> ─────────────────────────╮
│ A high level console interface.                                                  │
│                                                                                  │
│ ╭──────────────────────────────────────────────────────────────────────────────╮ │
│ │ <console width=194 ColorSystem.TRUECOLOR>                                    │ │
│ ╰──────────────────────────────────────────────────────────────────────────────╯ │
│                                                                                  │
│     color_system = 'truecolor'                                                   │
│         encoding = 'utf-8'                                                       │
│             file = <_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'> │
│           height = 72                                                            │
│    is_alt_screen = False                                                         │
│ is_dumb_terminal = False                                                         │
│   is_interactive = True                                                          │
│       is_jupyter = False                                                         │
│      is_terminal = True                                                          │
│   legacy_windows = False                                                         │
│         no_color = False                                                         │
│          options = ConsoleOptions(                                               │
│                        size=ConsoleDimensions(width=194, height=72),             │
│                        legacy_windows=False,                                     │
│                        min_width=1,                                              │
│                        max_width=194,                                            │
│                        is_terminal=True,                                         │
│                        encoding='utf-8',                                         │
│                        max_height=72,                                            │
│                        justify=None,                                             │
│                        overflow=None,                                            │
│                        no_wrap=False,                                            │
│                        highlight=None,                                           │
│                        markup=None,                                              │
│                        height=None                                               │
│                    )                                                             │
│            quiet = False                                                         │
│           record = False                                                         │
│         safe_box = True                                                          │
│             size = ConsoleDimensions(width=194, height=72)                       │
│        soft_wrap = False                                                         │
│           stderr = False                                                         │
│            style = None                                                          │
│         tab_size = 8                                                             │
│            width = 194                                                           │
╰──────────────────────────────────────────────────────────────────────────────────╯
╭─── <class 'rich._windows.WindowsConsoleFeatures'> ────╮
│ Windows features available.                           │
│                                                       │
│ ╭───────────────────────────────────────────────────╮ │
│ │ WindowsConsoleFeatures(vt=False, truecolor=False) │ │
│ ╰───────────────────────────────────────────────────╯ │
│                                                       │
│ truecolor = False                                     │
│        vt = False                                     │
╰───────────────────────────────────────────────────────╯
╭────── Environment Variables ───────╮
│ {                                  │
│     'TERM': 'xterm-256color',      │
│     'COLORTERM': 'truecolor',      │
│     'CLICOLOR': '1',               │
│     'NO_COLOR': None,              │
│     'TERM_PROGRAM': 'iTerm.app',   │
│     'COLUMNS': None,               │
│     'LINES': None,                 │
│     'JPY_PARENT_PID': None,        │
│     'VSCODE_VERBOSE_LOGGING': None │
│ }                                  │
╰────────────────────────────────────╯
platform="Darwin"

and:

$ pip freeze | grep rich
rich==12.4.1
@willmcgugan
Copy link
Collaborator

That was a very thorough assessment. I think the issue is that Rule is missing a __rich_measure__ method, which means that Table will default to giving it as much space as possible.

If you were to add a __rich_measure__ method that returned Measurement(1, 1) I would expect it to behave more like you would expect.

It hadn't come up, so I guess nobody has tried putting a rule in a table.

Would you like to submit a PR, since you got this far?

@gimbo
Copy link
Contributor Author

gimbo commented May 10, 2022

That was a very thorough assessment.

Thanks. 😄 Partially this is just me finally learning how to properly use rich beyond just rich.pretty.

...

Would you like to submit a PR, since you got this far?

I'll take a look. I'm short on time but it sounds like a straightforward fix...

gimbo added a commit to gimbo/__forked__rich that referenced this issue May 24, 2022
gimbo added a commit to gimbo/__forked__rich that referenced this issue May 24, 2022
gimbo added a commit to gimbo/__forked__rich that referenced this issue May 24, 2022
@willmcgugan
Copy link
Collaborator

Closing, assumed fixed.

Copy link

github-actions bot commented Jul 2, 2024

I hope we solved your problem.

If you like using Rich, you might also enjoy Textual

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

2 participants