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

exp show: customize rich table rendering behavior #5381

Merged
merged 4 commits into from
Feb 9, 2021

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented Feb 2, 2021

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

Will close #5247

  • Updates rich version to 9.x, uses native rich pager support
  • Re-enables color/styling in exp show table
  • When using --no-pager, columns will be collapsed in right-to-left order if the table cannot fit into the console
    • At least one metric column and one params column will always be displayed whenever possible.
    • Params will be collapsed before metrics
    • If the table still does not fit after collapsing all but one metric and param column, will fallback to rich's folding behavior for the remaining columns

TODO

  • Wrap rich column/table classes to collapse/merge metrics & params columns which can't be displayed due to terminal width

@pmrowla pmrowla added ui user interface / interaction skip-changelog Skips changelog labels Feb 2, 2021
@pmrowla pmrowla self-assigned this Feb 2, 2021
@pmrowla
Copy link
Contributor Author

pmrowla commented Feb 3, 2021

Collapse behavior example using jameson-metrics:

current master:

$ dvc exp show --no-pager
┏━━━━━━━━━━━━┳━┳━━━━━━━━━┳━━━━━┳━━━━━┳━━━━━━━━┳━━━━━━━━━┳━┳━┳━┳━┳━┳━┳
┃ Experiment ┃ ┃     AUC ┃  TP ┃  TN ┃   took ┃    loss ┃ ┃ ┃ ┃ ┃ ┃ ┃
┑━━━━━━━━━━━━╇━╇━━━━━━━━━╇━━━━━╇━━━━━╇━━━━━━━━╇━━━━━━━━━╇━╇━╇━╇━╇━╇━╇
β”‚ workspace  β”‚ β”‚ 0.75265 β”‚ 190 β”‚ 894 β”‚ 206.43 β”‚ 0.85513 β”‚ β”‚ β”‚ β”‚ β”‚ β”‚ β”‚
β”‚ master     β”‚ β”‚ 0.75265 β”‚ 190 β”‚ 894 β”‚ 206.43 β”‚ 0.85513 β”‚ β”‚ β”‚ β”‚ β”‚ β”‚ β”‚
└────────────┴─┴─────────┴─────┴─────┴────────┴─────────┴─┴─┴─┴─┴─┴─┴

after this PR:

$ dvc exp show --no-pager
┏━━━━━━━━━━━━┳━━━━━━━━━━━━━━┳━━━━━━━━━┳━━━━━┳━━━━━┳━━━┳━━━━━━━━┳━━━┓
┃ Experiment ┃ Created      ┃     AUC ┃  TP ┃  TN ┃ … ┃ L4size ┃ … ┃
┑━━━━━━━━━━━━╇━━━━━━━━━━━━━━╇━━━━━━━━━╇━━━━━╇━━━━━╇━━━╇━━━━━━━━╇━━━┩
β”‚ workspace  β”‚ -            β”‚ 0.75265 β”‚ 190 β”‚ 894 β”‚ … β”‚ 24     β”‚ … β”‚
β”‚ master     β”‚ Feb 29, 2020 β”‚ 0.75265 β”‚ 190 β”‚ 894 β”‚ … β”‚ 24     β”‚ … β”‚
β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”˜

@dmpetrov @dberenbaum does this work for collapsing the --no-pager table to fit terminal width?

@dberenbaum
Copy link
Collaborator

@pmrowla I like your idea to show the first listed metrics and parameters rather than let rich decide, and as you mentioned, maybe the first metric should even be on the right to be next to the first parameter and consistent with viewer.

A couple small suggestions to try (interested in feedback on whether others find these useful @pmrowla @dmpetrov @skshetry):

  • Provide a hint above or below the table with something like: 6 of 14 columns shown. Use --include-metrics/--include-params to specify which columns to show.
  • A suggestion from #8 in https://medium.com/@jdxcode/12-factor-cli-apps-dd3c227a0e46 is: "Never output table borders. It’s noisy and a huge pain for parsing." This would at least solve the issue of the empty columns:
  Experiment   Created            AUC    TP    TN     L4size    

  workspace    -              0.75265   190   894     24        
  master       Feb 29, 2020   0.75265   190   894     24        

One bigger suggestion is to provide a csv or tsv output option for a few reasons:

  • Users likely will have no problem reading csv files into other tools and formatting, sorting, and visualizing however they want, giving users more flexibility and making it less necessary to solve all those issues in cli output.
  • Number of columns does not need to be limited to screen width, so the leftmost column that summarizes a complex tree structure can be expanded into multiple columns, one for each level of the tree (and probably at least one additional one for checkpoints). Even rows that are mostly empty are fine, so more info can be provided by adding columns.
  • Provides an easily parseable serialization format for saving, sharing, etc.

@pmrowla
Copy link
Contributor Author

pmrowla commented Feb 8, 2021

This seems a bit more personal preference? Not having borders makes it harder to read bigger tables imo, but I don't have any strong opinions about it either way.

I do think that we shouldn't worry about making the table machine-parsable though. If users want machine readable data they should just be using a better output format like json (or potentially csv).

@pmrowla pmrowla changed the title [WIP] exp show: customize rich table rendering behavior exp show: customize rich table rendering behavior Feb 8, 2021
@pmrowla pmrowla changed the title exp show: customize rich table rendering behavior [WIP] exp show: customize rich table rendering behavior Feb 8, 2021
@pmrowla pmrowla marked this pull request as ready for review February 8, 2021 06:35
@pmrowla
Copy link
Contributor Author

pmrowla commented Feb 8, 2021

Will update table class location once #5348 is merged

@dberenbaum dberenbaum mentioned this pull request Feb 8, 2021
31 tasks
@pmrowla pmrowla changed the title [WIP] exp show: customize rich table rendering behavior exp show: customize rich table rendering behavior Feb 9, 2021
@pmrowla pmrowla merged commit b370465 into iterative:master Feb 9, 2021
@pmrowla pmrowla deleted the exp-rich branch February 9, 2021 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skips changelog ui user interface / interaction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

experiments: checkpoints cleanup/research issues
2 participants