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

Reducing the number of ---- printed at the top of skim() #424

Closed
ismayc opened this issue Mar 25, 2019 · 16 comments
Closed

Reducing the number of ---- printed at the top of skim() #424

ismayc opened this issue Mar 25, 2019 · 16 comments

Comments

@ismayc
Copy link

ismayc commented Mar 25, 2019

Is there a way to reduce the number of ──────── that appear to be more like the following instead of what is shown at https://github.com/ropensci/skimr/blob/master/README.md#presentation-is-in-a-compact-horizontal-format?

    skim(iris)

    ## Skim summary statistics
    ##  n obs: 150 
    ##  n variables: 5 
    ## 
    ## ── Variable type:factor ───────────────────────────────────────
    ##  variable missing complete   n n_unique                       top_counts ordered
    ##   Species       0      150 150        3 set: 50, ver: 50, vir: 50, NA: 0   FALSE
    ## 
    ## ── Variable type:numeric ──────────────────────────────────────
    ##      variable missing complete   n mean   sd  p0 p25  p50 p75 p100     hist
    ##  Petal.Length       0      150 150 3.76 1.77 1   1.6 4.35 5.1  6.9 ▇▁▁▂▅▅▃▁
    ##   Petal.Width       0      150 150 1.2  0.76 0.1 0.3 1.3  1.8  2.5 ▇▁▁▅▃▃▂▂
    ##  Sepal.Length       0      150 150 5.84 0.83 4.3 5.1 5.8  6.4  7.9 ▂▇▅▇▆▅▂▂
    ##   Sepal.Width       0      150 150 3.06 0.44 2   2.8 3    3.3  4.4 ▁▂▅▇▃▂▁▁
@elinw
Copy link
Collaborator

elinw commented Mar 26, 2019

Basically in the console this is an arbitrary length since different people have different settings that may impact display. The whole thing is opinionated. If you are knitting to HTML you could use CSS to manage this. I'd be willing to look at a PR about this although I'm not sure how it would work or what @michaelquinn32 would think ... but the PR should be against the v2 branch since we are winding down V1. Many aspects of printing have been changed in V2.

@ismayc
Copy link
Author

ismayc commented Mar 26, 2019

I'm happy to update to V2 if this is more easily tweaked there. I'll test it out in the meantime.

@ismayc
Copy link
Author

ismayc commented Mar 26, 2019

Maybe adding ... to the cli::rule() call could help here:

top_line <- cli::rule(line = 1, left = variable_type)

Then width could be specified to be different than console_width() as desired?

@elinw
Copy link
Collaborator

elinw commented Mar 30, 2019

I looked at this and cli::rule() does not have a ... option so that won't work. We could explicitly add a rule_width parameter to print and then set the cli::rule() width to that. That's kind of messy in general. But I think the real question is what are you trying to achieve? What's the point of having a width that is narrower than the printed area? To me I would have been more likely to say the issue is that when the console is wider than the printed area the rule is full width. But some people might prefer that.

We also spent quite a bit of time with @jenniferthompson who as one of our peer reviewers at ROpenSci talking about the layout , and that is how we came up with the design we have currently.

Now that v2 will be simplifying the printing substantially I'm wondering if it's better to think about a larger project that allows more customization in the print methods for those who want to do that. I can think of a few different things people might want to do if they are using skimr in specific situations (for example thousands of variables or creating data tables for journals that have specific styles).

@ismayc
Copy link
Author

ismayc commented Apr 1, 2019

I’m trying to get the output to work nicely in bookdown. Currently when printing to PDF with the console output it prints all the way into the right hand margin. skimr::kable() doesn’t work exactly how’d I’d like either with bookdown. I can work on a quick fix solution in a fork if that’s easier.

@michaelquinn32
Copy link
Collaborator

michaelquinn32 commented Apr 2, 2019 via email

@elinw
Copy link
Collaborator

elinw commented Apr 10, 2019

I do think it will be better in v2 but I also think we need to really look at margin handling in knitr when there are many statistics. One big reason for people to use pander is the nice table wrapping, including repeating the row labels.

@GegznaV
Copy link

GegznaV commented May 18, 2019

Just an example, where a smaller number of hyphens --- is preferred:
image

@elinw
Copy link
Collaborator

elinw commented Jul 13, 2019

If anything that should wrap to be above the second part of the table. We really ultimately need to think about the row splitting issue more carefully although it's kind of a big job. What I might do in v2 is to make a given table a bit more bespoke and take the type table and split it into pieces deliberately.

@elinw
Copy link
Collaborator

elinw commented Jul 13, 2019

See #473

@elinw
Copy link
Collaborator

elinw commented Dec 30, 2019

So is the basic goal this?
If in bookdown respect the right hand margin.
If in console respect the console width even though the max line length is set to Inf.

@ismayc
Copy link
Author

ismayc commented Dec 30, 2019

How about an option to just turn off the hyphens with a parameter? Those that have had issues with it could just switch it off as needed.

ModernDive has been published to print using modified output to remove the hyphens, so I’m not as keen on figuring out a solution here given it was six months ago when I opened this and we had to decide how to work with the output. Feel free to keep this closed if you like.

@elinw
Copy link
Collaborator

elinw commented Jan 1, 2020

So interestingly cli::rule() has a width option that defaults to the base::options() value. For bookdown specifically it would probably make sense to set this globally.

@ismayc
Copy link
Author

ismayc commented Jan 1, 2020

I don't think I was clear enough but this was what I was trying to say in the comment above: #424 (comment)

@elinw
Copy link
Collaborator

elinw commented Jan 1, 2020

Right but we can't arbitrarily add an option to a function from another package. That breaks the rules on inheritance.
We have two different widths in operation here. One is the value for cli::rule() from base::options() and the other is width = Inf from the print.skim_df() function that makes it so the tibbles do not wrap. I'm wondering if we can add a second option there since it is already kind of option heavy and one more won't matter. (We can do that because print() does have ...).

The other idea I have is to have an option for printing with no rules at all. I think that makes sense in the context of html and pdf now that we are using

@elinw
Copy link
Collaborator

elinw commented Jan 2, 2020

Okay here is a pull request.
#561
You would use it like this:
skim(iris) %>% print(rule_width = 20)

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