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

Closes #151 - Metadata includes verbose option #199

Merged
merged 18 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## New Features and Bug Fixes

* `xportr_metadata()` can set `verbose` for a whole pipeline
EeethB marked this conversation as resolved.
Show resolved Hide resolved

## Documentation

* `xportr_write()` now accepts `metadata` argument which can be used to set the dataset label to stay consistent with the other `xportr_*` functions. It is noteworthy that the dataset label set using the `xportr_df_label()` function will be retained during the `xportr_write()`.
* Exporting a new dataset `dataset_spec` that contains the Dataset Specification for ADSL. (#179)

Expand Down
8 changes: 7 additions & 1 deletion R/label.R
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
xportr_label <- function(.df,
metadata = NULL,
domain = NULL,
verbose = getOption("xportr.label_verbose", "none"),
verbose = NULL,
metacore = deprecated()) {
if (!missing(metacore)) {
lifecycle::deprecate_warn(
Expand Down Expand Up @@ -101,6 +101,12 @@ xportr_label <- function(.df,
# Check any variables missed in metadata but present in input data ---
miss_vars <- setdiff(names(.df), metadata[[variable_name]])

# Verbose should use an explicit verbose option first, then the value set in
# metadata, and finally fall back to the option value
verbose <- verbose %||%
attr(.df, "_xportr.df_verbose_") %||%
getOption("xportr.label_verbose", "none")

label_log(miss_vars, verbose)

label <- metadata[[variable_label]]
Expand Down
8 changes: 7 additions & 1 deletion R/length.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
xportr_length <- function(.df,
metadata = NULL,
domain = NULL,
verbose = getOption("xportr.length_verbose", "none"),
verbose = NULL,
metacore = deprecated()) {
if (!missing(metacore)) {
lifecycle::deprecate_warn(
Expand Down Expand Up @@ -108,6 +108,12 @@ xportr_length <- function(.df,
# Check any variables missed in metadata but present in input data ---
miss_vars <- setdiff(names(.df), metadata[[variable_name]])

# Verbose should use an explicit verbose option first, then the value set in
# metadata, and finally fall back to the option value
verbose <- verbose %||%
attr(.df, "_xportr.df_verbose_") %||%
getOption("xportr.length_verbose", "none")

length_log(miss_vars, verbose)

length <- metadata[[variable_length]]
Expand Down
10 changes: 8 additions & 2 deletions R/metadata.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@
#' xportr_type() %>%
#' xportr_order()
#' }
xportr_metadata <- function(.df, metadata, domain = NULL) {
xportr_metadata <- function(.df,
metadata,
domain = NULL,
verbose = NULL) {
## Common section to detect domain from argument or pipes

df_arg <- tryCatch(as_name(enexpr(.df)), error = function(err) NULL)
Expand All @@ -46,5 +49,8 @@ xportr_metadata <- function(.df, metadata, domain = NULL) {

## End of common section

structure(.df, `_xportr.df_metadata_` = metadata)
structure(.df,
Copy link
Collaborator

@averissimo averissimo Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very very optional. I would think that {styler} or {lintr} didn't like this indentation

Suggested change
structure(.df,
structure(
.df,

`_xportr.df_metadata_` = metadata,
`_xportr.df_verbose_` = verbose
Comment on lines +64 to +65
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use quotes ", instead of backticks

Suggested change
`_xportr.df_metadata_` = metadata,
`_xportr.df_verbose_` = verbose
"_xportr.df_metadata_" = metadata,
"_xportr.df_verbose_" = verbose

)
}
8 changes: 7 additions & 1 deletion R/order.R
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
xportr_order <- function(.df,
metadata = NULL,
domain = NULL,
verbose = getOption("xportr.order_verbose", "none"),
verbose = NULL,
metacore = deprecated()) {
if (!missing(metacore)) {
lifecycle::deprecate_warn(
Expand Down Expand Up @@ -122,6 +122,12 @@ xportr_order <- function(.df,
# Used in warning message for how many vars have been moved
reorder_vars <- names(df_re_ord)[names(df_re_ord) != names(.df)]

# Verbose should use an explicit verbose option first, then the value set in
# metadata, and finally fall back to the option value
verbose <- verbose %||%
attr(.df, "_xportr.df_verbose_") %||%
getOption("xportr.order_verbose", "none")

# Function is located in messages.R
var_ord_msg(reorder_vars, names(drop_vars), verbose)

Expand Down
8 changes: 7 additions & 1 deletion R/type.R
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
xportr_type <- function(.df,
metadata = NULL,
domain = NULL,
verbose = getOption("xportr.type_verbose", "none"),
verbose = NULL,
metacore = deprecated()) {
if (!missing(metacore)) {
lifecycle::deprecate_warn(
Expand Down Expand Up @@ -148,6 +148,12 @@ xportr_type <- function(.df,
type.y = if_else(type.y %in% numericTypes, "_numeric", type.y)
)

# Verbose should use an explicit verbose option first, then the value set in
# metadata, and finally fall back to the option value
verbose <- verbose %||%
attr(.df, "_xportr.df_verbose_") %||%
getOption("xportr.type_verbose", "none")

# It is possible that a variable exists in the table that isn't in the metadata
# it will be silently ignored here. This may happen depending on what a user
# passes and the options they choose. The check_core function is the place
Expand Down
2 changes: 1 addition & 1 deletion README.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ The `xportr_metadata()` function can reduce duplication by setting the variable

```{r, message=FALSE, eval=FALSE}
adsl %>%
xportr_metadata(var_spec, "ADSL") %>%
xportr_metadata(var_spec, "ADSL", verbose = "warn") %>%
xportr_type() %>%
xportr_length() %>%
xportr_label() %>%
Expand Down
2 changes: 1 addition & 1 deletion man/xportr_label.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/xportr_length.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion man/xportr_metadata.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/xportr_order.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/xportr_type.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions tests/testthat/test-length.R
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,13 @@ test_that("xportr_length: Impute character lengths based on class", {
expect_message("Variable lengths missing from metadata") %>%
expect_message("lengths resolved") %>%
expect_attr_width(c(7, 199, 200, 200, 8))

adsl %>%
xportr_metadata(metadata, verbose = "none") %>%
xportr_length() %>%
expect_message("Variable lengths missing from metadata") %>%
expect_message("lengths resolved") %>%
expect_attr_width(c(7, 199, 200, 200, 8))
})

test_that("xportr_length: Throws message when variables not present in metadata", {
Expand All @@ -144,6 +151,12 @@ test_that("xportr_length: Throws message when variables not present in metadata"
expect_message("Variable lengths missing from metadata") %>%
expect_message("lengths resolved") %>%
expect_message(regexp = "Problem with `y`")

xportr_metadata(adsl, metadata, verbose = "message") %>%
xportr_length() %>%
expect_message("Variable lengths missing from metadata") %>%
expect_message("lengths resolved") %>%
expect_message(regexp = "Problem with `y`")
})

test_that("xportr_length: Metacore instance can be used", {
Expand Down
13 changes: 13 additions & 0 deletions tests/testthat/test-order.R
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,19 @@ test_that("xportr_order: Variable ordering messaging is correct", {
expect_message("2 variables not in spec and moved to end") %>%
expect_message("Variable moved to end in `.df`: `a` and `z`") %>%
expect_message("All variables in dataset are ordered")

# Metadata versions
xportr_metadata(df, df_meta, verbose = "message") %>%
xportr_order() %>%
expect_message("All variables in specification file are in dataset") %>%
expect_condition("4 reordered in dataset") %>%
expect_message("Variable reordered in `.df`: `a`, `b`, `c`, and `d`")

xportr_metadata(df2, df_meta, verbose = "message") %>%
xportr_order() %>%
expect_message("2 variables not in spec and moved to end") %>%
expect_message("Variable moved to end in `.df`: `a` and `z`") %>%
expect_message("All variables in dataset are ordered")
})

test_that("xportr_order: Metadata order columns are coersed to numeric", {
Expand Down
20 changes: 16 additions & 4 deletions tests/testthat/test-type.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ test_that("xportr_type: Variable types are coerced as expected and can raise mes
(df3 <- suppressMessages(xportr_type(df, meta_example, verbose = "warn"))) %>%
expect_warning()

# Metadata version of the last statement
df %>%
xportr_metadata(meta_example, verbose = "warn") %>%
xportr_type() %>%
expect_warning()

expect_equal(purrr::map_chr(df3, class), c(
Subj = "numeric", Different = "character",
Val = "numeric", Param = "character"
Expand All @@ -77,6 +83,12 @@ test_that("xportr_type: Variable types are coerced as expected and can raise mes
expect_message("Variable type\\(s\\) in dataframe don't match metadata")
)

# Metadata version
df %>%
xportr_metadata(meta_example, verbose = "message") %>%
xportr_type() %>%
expect_message("Variable type\\(s\\) in dataframe don't match metadata")

expect_equal(purrr::map_chr(df4, class), c(
Subj = "numeric", Different = "character",
Val = "numeric", Param = "character"
Expand All @@ -100,12 +112,12 @@ test_that("xportr_metadata: Var types coerced as expected and raise messages", {
))

suppressMessages(
xportr_metadata(df, meta_example) %>% xportr_type(verbose = "stop")
xportr_metadata(df, meta_example, verbose = "stop") %>% xportr_type()
) %>%
expect_error()

suppressMessages(
df3 <- xportr_metadata(df, meta_example) %>% xportr_type(verbose = "warn")
df3 <- xportr_metadata(df, meta_example, verbose = "warn") %>% xportr_type()
) %>%
expect_warning()

Expand All @@ -116,8 +128,8 @@ test_that("xportr_metadata: Var types coerced as expected and raise messages", {

suppressMessages({
(
df4 <- xportr_metadata(df, meta_example) %>%
xportr_type(verbose = "message")
df4 <- xportr_metadata(df, meta_example, verbose = "message") %>%
xportr_type()
) %>%
expect_message("Variable type\\(s\\) in dataframe don't match metadata: `Subj` and `Val`")
})
Expand Down
Loading