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

Adaptive rendering doesn't fully work #331

Closed
jennybc opened this issue Jun 25, 2021 · 6 comments · Fixed by #348
Closed

Adaptive rendering doesn't fully work #331

jennybc opened this issue Jun 25, 2021 · 6 comments · Fixed by #348
Milestone

Comments

@jennybc
Copy link
Member

jennybc commented Jun 25, 2021

I am following the latlon example in Printing vectors nicely in tibbles.
In particular, I want to do adaptive rendering.

In my real example, I have a character vector of ugly strings that mean almost nothing to humans (Google Drive file ids, but think: Git commit SHAs or similar).
If there is any sort of horizontal space shortage, this column should be heavily truncated. However I don't, in general, want to truncate this object. I don't want to truncate in a format method. Only inside a tibble.

I'll use the toy foo class to demonstrate.

library(tidyverse)
library(vctrs)
library(pillar)

new_foo <- function(x = character()) {
  vec_assert(x, character())
  new_vctr(x, class = "foo")
}

data <- tibble(
  month = month.name[1:3],
  sentences = new_foo(stringr::sentences[1:3]),
  blah = stringr::sentences[4:6]
)
data
#> # A tibble: 3 x 3
#>   month   sentences                                   blah                      
#>   <chr>   <foo>                                       <chr>                     
#> 1 January The birch canoe slid on the smooth planks.  These days a chicken leg …
#> 2 Februa… Glue the sheet to the dark blue background. Rice is often served in r…
#> 3 March   It's easy to tell the depth of a well.      The juice of lemons makes…
print(data, width = 60)
#> # A tibble: 3 x 3
#>   month  sentences                                   blah   
#>   <chr>  <foo>                                       <chr>  
#> 1 Janua… The birch canoe slid on the smooth planks.  These …
#> 2 Febru… Glue the sheet to the dark blue background. Rice i…
#> 3 March  It's easy to tell the depth of a well.      The ju…

Here we see that the foo column is hogging all the space, because it never gets truncated, because I haven't implemented any methods that would allow this.

If I implement a pillar_shaft method as the vignette shows, I do, in fact get truncation. However the horizontal space gained isn't redistributed to the other columns.

pillar_shaft.foo <- function(x, ...) {
  full <- format(x)
  trunc <- format(paste0(substr(x, 1, 7), cli::symbol$continue))
  pillar::new_pillar_shaft(
    list(full = full, trunc = trunc),
    width = pillar::get_max_extent(full),
    min_width = pillar::get_max_extent(trunc),
    class = "pillar_shaft_foo"
  )
}

format.pillar_shaft_foo <- function(x, width, ...) {
  if (pillar::get_max_extent(x$full) <= width) {
    ornament <- x$full
  } else {
    ornament <- x$trunc
  }

  pillar::new_ornament(ornament, align = "left")
}

data
#> # A tibble: 3 x 3
#>   month    sentences                             blah                              
#>   <chr>    <foo>                                 <chr>                             
#> 1 January  The bir…                              These days a chicken leg is a rar…
#> 2 February Glue th…                              Rice is often served in round bow…
#> 3 March    It's ea…                              The juice of lemons makes fine pu…
print(data, width = 60)
#> # A tibble: 3 x 3
#>   month   sentences                   blah                  
#>   <chr>   <foo>                       <chr>                 
#> 1 January The bir…                    These days a chicken …
#> 2 Februa… Glue th…                    Rice is often served …
#> 3 March   It's ea…                    The juice of lemons m…

Having poked around a bit, it seems like the logic in colonnade_distribute_space_df() isn't really taking advantage of the fact that the foo column only has width 8 if it is truncated at all.

Is there any way for me to do this?

If we can't display all columns fully, then foo should use its truncated form. And all the freed horizontal space should be available to the remaining columns.

@krlmlr
Copy link
Member

krlmlr commented Jun 27, 2021

Thanks. The issue here seems to be that any unused width is not rebalanced. I'm not sure if this even was the case with pillar 1.4.7 (needs a compatible tibble version from around that date).


author: kirill
date: 2021-06-27
output: "reprex::reprex\_document"
title: mid-guppy_reprex.R

library(tidyverse)
library(vctrs)
#> 
#> Attaching package: 'vctrs'
#> The following object is masked from 'package:dplyr':
#> 
#>     data_frame
#> The following object is masked from 'package:tibble':
#> 
#>     data_frame
library(pillar)
#> 
#> Attaching package: 'pillar'
#> The following object is masked from 'package:dplyr':
#> 
#>     dim_desc

new_foo <- function(x = character()) {
  vec_assert(x, character())
  new_vctr(x, class = "foo")
}

data <- tibble(
  month = month.name[1:3],
  sentences = new_foo(stringr::sentences[1:3]),
  blah = stringr::sentences[4:6]
)

pillar_shaft.foo <- function(x, ...) {
  full <- format(x)
  pillar::new_pillar_shaft(
    list(x = x, full = full),
    width = pillar::get_max_extent(full),
    min_width = 8,
    class = "pillar_shaft_foo"
  )
}

format.pillar_shaft_foo <- function(x, width, ...) {
  if (pillar::get_max_extent(x$full) <= width) {
    ornament <- x$full
  } else {
    ornament <- format(paste0(fansi::substr2_sgr(x$full, 1, width, type = "width") , cli::symbol$continue))
  }
  
  pillar::new_ornament(ornament, align = "left")
}

data
#> # A tibble: 3 x 3
#>   month    sentences                            blah                            
#>   <chr>    <foo>                                <chr>                           
#> 1 January  The birch canoe slid on the smooth p… These days a chicken leg is a r…
#> 2 February Glue the sheet to the dark blue back… Rice is often served in round b…
#> 3 March    It's easy to tell the depth of a wel… The juice of lemons makes fine …
print(data[-1], width = 60)
#> # A tibble: 3 x 2
#>   sentences                       blah                      
#>   <foo>                           <chr>                     
#> 1 The birch canoe slid on the smo… These days a chicken leg …
#> 2 Glue the sheet to the dark blue… Rice is often served in r…
#> 3 It's easy to tell the depth of … The juice of lemons makes…

Created on 2021-06-27 by the reprex package (v2.0.0)

One solution would be to measure the actual width and then rebalance each tier so that columns are shown next to each other and still use the available width.

@krlmlr
Copy link
Member

krlmlr commented Jun 27, 2021

This seems consistent with your observations.

Also, in the original example the consumed width seems too wide, I'm not sure why width = 80 gives a width of 83 characters.

@krlmlr
Copy link
Member

krlmlr commented Jul 24, 2021

re width = 83: I counted #> 🤦

I think I have a good idea how to achieve this. In the first iteration, we rebalance each tier individually; later we could consider rebalancing across tiers to minimize surprise.

@krlmlr
Copy link
Member

krlmlr commented Jul 25, 2021

Very soon:

library(tidyverse)
library(vctrs)
library(pillar)

new_foo <- function(x = character()) {
  vec_assert(x, character())
  new_vctr(x, class = "foo")
}

data <- tibble(
  month = month.name[1:3],
  sentences = new_foo(stringr::sentences[1:3]),
  blah = stringr::sentences[4:6]
)

pillar_shaft.foo <- function(x, ...) {
  full <- format(x)
  trunc <- format(paste0(substr(x, 1, 7), cli::symbol$continue))
  pillar::new_pillar_shaft(
    list(full = full, trunc = trunc),
    width = pillar::get_max_extent(full),
    min_width = pillar::get_max_extent(trunc),
    class = "pillar_shaft_foo"
  )
}

format.pillar_shaft_foo <- function(x, width, ...) {
  if (pillar::get_max_extent(x$full) <= width) {
    ornament <- x$full
  } else {
    ornament <- x$trunc
  }

  pillar::new_ornament(ornament, align = "left")
}

data
#> # A tibble: 3 × 3
#>   month    sentences                                   blah                     
#>   <chr>    <foo>                                       <chr>                    
#> 1 January  The birch canoe slid on the smooth planks.  These days a chicken leg…
#> 2 February Glue the sheet to the dark blue background. Rice is often served in …
#> 3 March    It's easy to tell the depth of a well.      The juice of lemons make…
options(width = 50)
print(data)
#> # A tibble: 3 × 3
#>   month    sentences blah                         
#>   <chr>    <foo>     <chr>                        
#> 1 January  The bir…  These days a chicken leg is …
#> 2 February Glue th…  Rice is often served in roun…
#> 3 March    It's ea…  The juice of lemons makes fi…

Created on 2021-07-25 by the reprex package (v2.0.0.9000)

This took way way longer than anticipated, but it's good for the codebase and for usability.

krlmlr added a commit that referenced this issue Jul 26, 2021
- If a column doesn't make use of all horizontal width offered to it, the excess width is distributed over other columns (#331).
@jennybc
Copy link
Member Author

jennybc commented Jul 26, 2021

Thanks, I look forward to taking advantage of this! In my life, at least (Drive ids, Git SHAs), I can think of many potential vctrs classes that are suitable for this sort of treatment.

This took way way longer than anticipated, but it's good for the codebase and for usability.

I thought it was really hard, so am pleasantly surprised to already have a fix.

@github-actions
Copy link
Contributor

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants