feat: Modern cli-styled print output #2682
Conversation
maelle
left a comment
maelle
left a comment
There was a problem hiding this comment.
Awesome to see this PR! Some first comments.
|
Need to discuss if we make this default or not. Probably for 3.0.0 a good idea but this will most definitely break a lot of snapshot tests everywhere 😆 |
This hasn't stopped others from updating their packages, so I wouldn't feel bad about this. |
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 9528357 is merged into main: |
Add detailed documentation explaining the dual layout modes (detailed vs compact) for vertex and edge sequence printing, clarify the role of the "single" attribute flag, and replace `identical(full, TRUE)` / `is.logical(full) && full` with `isTRUE(full)` for consistency.
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if a72b769 is merged into main: |
… semantic classes (`.field`, `.cls`) instead of hard-coded colors
…s, edge formatting, and line printing
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if f68ce31 is merged into main: |
Add tests/testthat/setup.R pinning `print.style = "classic"` and `print.id = FALSE` for the whole suite, so test output is deterministic and no longer depends on the package defaults (print.style now defaults to "cli"). cli output is exercised explicitly in test-print-cli.R. - Rename test-print.R -> test-print-classic.R (and its snapshot file) to mirror test-print-cli.R, and revert its content to the pre-branch state now that setup.R pins the style. - Drop the now-redundant per-test `print.style = "classic"` settings (test-games.R) and `print.id = FALSE` settings across the suite. test-par.R keeps its print.id settings as it tests the option itself. - Restore the snapshots that had churned to cli output back to classic; regenerate flow.md, where pinning print.id off removes a graph id that the test previously masked via transform. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make print.igraph.vs() and print.igraph.es() pure dispatchers: guard on `!is_cli_style()` to delegate to the new print_igraph_vs_legacy() / print_igraph_es_legacy() (the former inline classic bodies), and fall through to the cli renderer otherwise. cli is the default style now, so treating it as the main path and legacy as the special case reads more naturally and keeps both branches as named functions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the same dispatch inversion as the vs/es printers to print.igraph() and summary.igraph(): guard on `!is_cli_style()` to delegate to the new print_igraph_legacy() / summary_igraph_legacy() (the former inline classic bodies), and fall through to the cli renderer otherwise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
all comments should be addressed now 🙏 |
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if aa8609f is merged into main: |
3 participants
fix #1959
Modern printing with cli, currently guarded behind
igraph_options(print.style = "cli")where "classic" is the default old style.