rstudio / pointblank

Data quality assessment and metadata reporting for data frames and database tables
https://rstudio.github.io/pointblank/
Other
878 stars 56 forks source link

`incorporate()` fails with multi-element fields in `info_tabular()` #364

Open petrbouchal opened 2 years ago

petrbouchal commented 2 years ago

Description

When info_tabular() is provided a multi-element argument to one of its dots parameters, incorporate() fails with error All unnamed arguments must be length 1.

Without running incorporate(), the processing succeeds and the multi-element input is nicely turned into multiple lines in the output.

This can be worked around but the multi-element input is a succinct way of creating multi-line description fields, also making the source code more legible.

Reproducible example

library(pointblank)

# bug

x <- create_informant(tbl = mtcars,
                 label = "Codebook") %>%
  info_tabular(Info = c("Line 1",
                        "Line 2"),
               `Additional` = "Extra info") %>%

  info_snippet(
    snippet_name = "min_wt",
    fn = snip_lowest(column = "wt")
  ) %>%
  info_columns("wt",
               Popis = "weight",
               Min = "minimum is {min_wt}") %>%
  incorporate()
#> 

#> ── Incorporation Started - there is a single snippet to process ────────────────

#> ✓ Information gathered.

#> ✓ Snippets processed.

#> Error: All unnamed arguments must be length 1
# snippet and incorporate, single-element info

x <- create_informant(tbl = mtcars,
                 label = "Codebook") %>%
  info_tabular(Info = c("Line 1"),
               `Additional` = "Extra info") %>%
  info_snippet(
    snippet_name = "min_wt",
    fn = snip_lowest(column = "wt")
  ) %>%
  info_columns("wt",
               Popis = "weight",
               Min = "minimum is {min_wt}") %>%
  incorporate()
#> 

#> ── Incorporation Started - there is a single snippet to process ────────────────

#> ✓ Information gathered.

#> ✓ Snippets processed.

#> ✓ Information built.

#> 

#> ── Incorporation Completed ─────────────────────────────────────────────────────
# no snippet, no incorporate

x <- create_informant(tbl = mtcars,
                 label = "Codebook") %>%
  info_tabular(Info = c("Line 1",
                        "Line 2"),
               `Additional` = "Extra info") %>%
  info_columns("wt",
               Popis = "weight")

# no snippet, incorporate, two-element info

x <- create_informant(tbl = mtcars,
                 label = "Codebook") %>%
  info_tabular(Info = c("Line 1",
                        "Line 2"),
               `Additional` = "Extra info") %>%
  info_columns("wt",
               Popis = "weight") %>% 
  incorporate()
#> 

#> ── Incorporation Started ───────────────────────────────────────────────────────

#> ✓ Information gathered.

#> Error: All unnamed arguments must be length 1

Created on 2021-11-24 by the reprex package (v2.0.0)

Expected result

I would expect the pipeline to handle multi-element input identically regardless of whether snippets and incorporate() are part of the flow

Session info

R version 4.1.0 (2021-05-18) Platform: x86_64-apple-darwin17.0 (64-bit) Running under: macOS Big Sur 11.6.1

Matrix products: default LAPACK: /Library/Frameworks/R.framework/Versions/4.1/Resources/lib/libRlapack.dylib

locale: [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages: [1] stats graphics grDevices datasets utils methods base

other attached packages: [1] pointblank_0.9.0 forcats_0.5.0 stringr_1.4.0 dplyr_1.0.7
[5] purrr_0.3.4 readr_2.0.2 tidyr_1.1.3 tibble_3.1.3
[9] ggplot2_3.3.3 tidyverse_1.3.0

loaded via a namespace (and not attached): [1] httr_1.4.2 sass_0.4.0 jsonlite_1.7.2 here_1.0.1
[5] modelr_0.1.8 assertthat_0.2.1 highr_0.8 renv_0.14.0
[9] cellranger_1.1.0 yaml_2.2.1 ptrr_0.2.0.9000 gdtools_0.2.3
[13] Rttf2pt1_1.3.8 pillar_1.6.2 backports_1.2.1 glue_1.4.2
[17] extrafontdb_1.0 digest_0.6.27 checkmate_2.0.0 rvest_0.3.6
[21] colorspace_2.0-0 htmltools_0.5.2 hrbrthemes_0.8.0 clipr_0.7.1
[25] pkgconfig_2.0.3 broom_0.7.3 haven_2.3.1 scales_1.1.1
[29] processx_3.5.2 tzdb_0.1.2 styler_1.5.1 generics_0.1.0
[33] ellipsis_0.3.2 withr_2.4.2 cli_3.0.1 magrittr_2.0.1
[37] crayon_1.4.2 readxl_1.3.1 evaluate_0.14 ps_1.6.0
[41] fs_1.5.0 fansi_0.5.0 xml2_1.3.2 tools_4.1.0
[45] data.table_1.14.0 hms_1.0.0 lifecycle_1.0.0 munsell_0.5.0
[49] reprex_2.0.0 targets_0.6.0.9000 callr_3.7.0.9000 compiler_4.1.0
[53] systemfonts_1.0.1 rlang_0.4.11 grid_4.1.0 gt_0.3.1
[57] rstudioapi_0.13 igraph_1.2.6 rmarkdown_2.11 gtable_0.3.0
[61] codetools_0.2-18 blastula_0.3.2 DBI_1.1.0 R6_2.5.0
[65] lubridate_1.7.10 knitr_1.36 fastmap_1.1.0 extrafont_0.17
[69] utf8_1.2.2 rprojroot_2.0.2 commonmark_1.7 stringi_1.5.3
[73] Rcpp_1.0.6 vctrs_0.3.8 dbplyr_2.1.1 tidyselect_1.1.1
[77] xfun_0.24

rich-iannone commented 2 years ago

I think this should be changed to work as you expect. Thanks for filing this issue!

petrbouchal commented 2 years ago

Thanks for this - unfortunately I'm still seeing the same result when running the first reprex using the current HEAD version (f6c16a99).

rich-iannone commented 2 years ago

I'm so sorry. I worded that terribly (and confused myself after re-reading it just now).

What I mean is that I ought to fix this because it's a worthy bug. This is what you get when you type something quickly on a mobile phone :(. But, this will be fixed soon. I'm even bumping up the priority on this now.

petrbouchal commented 2 years ago

No worries at all - re-reading this I think the correct interpretation of what you wrote is the more plausible one :) i.e. it was me misreading it. Apologies for jumping the gun (I just wanted to make sure I provide feedback and not drop the ball) and no hurry at all.