Changes from GOSL reporting

[paddytobias]

No description provided.

[paddytobias]

Hey @kinto-b , just making some changes to support GOSL reporting. Would you mind reviewing when you get a chance?

[kinto-b]

@paddytobias Yo!

Is the intention here to use the name provided in the shadow (for one-place shadows) as the name of the column? For example prj_project(blah, list(v=c(n="{N}")) would rename v to n in the projected df?

The only issue I can see is that providing the same named one-place shadow for multiple columns will now throw an error. For instance:

tbl <- mtcars %>% 
  prj_tbl_rows(
    Cylinders = cyl,
    Transmission = list(Automatic = am %in% 0, Manual = am %in% 1),
    ) %>% 
  prj_tbl_cols(
    `V-Shaped` = col_freq(n = vs %in% 1, N = vs %in% 0:1),
    `Not V-shaped` = col_freq(n = vs %in% 0, N = vs %in% 0:1)
    ) %>% 
  prj_tbl_summarise()
  
tbl %>% 
  prj_project(list(
    `V-Shaped` = c(p = "{signif(p, 2)} ({n})"),
    `Not V-shaped` = c(p = "{signif(p, 2)} ({n})")
  ))

It would be good to handle this behind the scenes and signal a warning.

Other than that, could you also add:

• a test case or two, and
• an example into the NEWS and the function @examples section to illustrate the new behaviour?

[paddytobias]

Thanks @kinto-b for thinking this through more, I've added some internal handling for duplicate column names. A increment suffix is added to the name and a warning is thrown

library(projectable)
library(magrittr)
#> Warning: package 'magrittr' was built under R version 4.0.5
tbl <- mtcars %>% 
  prj_tbl_rows(
    Cylinders = cyl,
    Transmission = list(Automatic = am %in% 0, Manual = am %in% 1),
  ) %>% 
  prj_tbl_cols(
    `V-Shaped` = col_freq(n = vs %in% 1, N = vs %in% 0:1),
    `Not V-shaped` = col_freq(n = vs %in% 0, N = vs %in% 0:1)
  ) %>% 
  prj_tbl_summarise()

tbl %>% 
  prj_project(list(
    `V-Shaped` = c(p = "{signif(p, 2)} ({n})"),
    `Not V-shaped` = c(p = "{signif(p, 2)} ({n})")
  ))
#> Warning in prj_cast_shadow(.data, .digits = .digits): 
#> Column name duplicated: `p`
#> Resolving by incrementing...
#> # A tibble: 5 x 4
#>   row_spanner  rows      p1        p2       
#> * <col_row>    <col_row> <glue>    <glue>   
#> 1 Cylinders    4         0.91 (10) 0.09 (1) 
#> 2 Cylinders    6         0.57 (4)  0.43 (3) 
#> 3 Cylinders    8         0 (0)     1 (14)   
#> 4 Transmission Automatic 0.37 (7)  0.63 (12)
#> 5 Transmission Manual    0.54 (7)  0.46 (6)

^{Created on 2022-05-27 by the reprex package (v2.0.1)}

Have also added to the tests and added examples to the NEWS and function documentation

[kinto-b]

Thanks Paddy :) Looking good! Just a small few comments

[kinto-b]

I don't think importing the dplyr dep is worth it for just this case_when and the one below. This can easily be changed to a series of assignments:

x[is.na(x) | is.na(N) | N %in% 0] <- ""
# etc.

[kinto-b]

I'm still not entirely convinced these belong here. They're much more related to how SRC does reporting than the projectable class and associated methods. Also would be a bit cleaner to introduce these in a separate PR so that they have their own commit

[justanna95]

Hey @paddytobias I've moved the public functions back to srcreporting and updated the versioning etc. I don't seem to have permissions to push my commits to github though.

[kinto-b]

Hey @justanna95, I've just given you permissions, so should be good to go :)