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

Feature request: Add + for rulesets #9

Merged
merged 7 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 8 additions & 3 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ Authors@R: c(
person(given = "David",
family = "Zimmermann-Kollenda",
role = c("aut","cre"),
email = "[email protected]"))
email = "[email protected]"),
person(given = "Beniamino",
family = "Green",
role = "ctb",
email = "[email protected]")
)
Description: Allows you to define rules which can be used to verify a given
dataset.
The package acts as a thin wrapper around more powerful data packages such
Expand All @@ -15,9 +20,9 @@ License: MIT + file LICENSE
URL: https://github.com/DavZim/dataverifyr,
https://davzim.github.io/dataverifyr/
BugReports: https://github.com/DavZim/dataverifyr/issues
Imports:
Imports:
yaml
Suggests:
Suggests:
arrow,
data.table,
DBI,
Expand Down
2 changes: 1 addition & 1 deletion NAMESPACE
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# Generated by roxygen2: do not edit by hand

S3method("+",rule)
S3method("+",ruleset)
S3method(print,rule)
S3method(print,ruleset)
export("%+%")
export(check_data)
export(detect_backend)
export(filter_fails)
Expand Down
48 changes: 39 additions & 9 deletions R/ruleset_construction.R
Original file line number Diff line number Diff line change
@@ -1,24 +1,54 @@
#' Add Rulesets Together
#' Add Rules and Rulesets Together
#'
#' `+` allows you to concatenate several rulesets into a larger rulesets. This
#' `+` allows you to add rules and rulesets into larger rulesets. This
#' can be useful if you want to create a ruleset for a dataset out of checks
#' for other datasets.
#'
#' @param a the first ruleset you wish to add
#' @param a the second ruleset you wish to add
#' @rdname ruleset_add
#' @export
"+.ruleset" <- function(a,b) {
out <- c(a,b)
class(out) <- "ruleset"
datavarifyr_plus <- function(a,b) {
# This is the method to add *both* rules and rulesets together.
# Has to be done this way AFAIK to comply with R's
# [double dispatch](https://yutani.rbind.io/post/double-dispatch-of-s3-method/)
# semantics.

if (class(a) == "ruleset" & class(b)=="ruleset") {
Copy link
Owner

@DavZim DavZim Oct 17, 2023

Choose a reason for hiding this comment

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

what about something like this?

if (class(a) != "ruleset") a <- list(a)
if (class(b) != "ruleset") b <- list(b)
out <- list(a, b)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't work in the case that both a and b are rules, but I like the suggestion - this could have been more concise.

This should work:

if (class(a) == "rule" & class(b) == "rule") {
    out <- list(a,b)
} else {
    if (class(a) == "rule") a <- list(a)
    if (class(b) == "rule") b <- list(b)
    out <- c(a, b)
}

out <- c(a,b)
} else if (class(a) == "ruleset" & class(b) == "rule"){
out <- c(a,list(b))
} else if (class(a) == "rule" & class(b) == "ruleset"){
out <- c(list(a),b)
} else{
out <- list(a,b)
}

for (i in seq_along(out)){
out[[i]]['index'] <- i
}

out
class(out) <- "ruleset"
return(out)
}

#' @rdname ruleset_add
#' @export
"%+%" <- `+.ruleset`
#' @rdname dataverifyr_add
"+.ruleset" <- datavarifyr_plus

#' @export
#' @rdname dataverifyr_add
"+.rule" <- datavarifyr_plus


###############################################################################

#' Programatically Combine a List of Rules and Rulesets into a Single Ruleset
#'
#' @param rule_ruleset_list a list of rules and rulesets you whish to combine
#' into a single list
#'
#' @return a ruleset which consolidates all the inputs
bind_rules <- function(rule_ruleset_list) {
Reduce(`+`, rule_ruleset_list)
}

18 changes: 18 additions & 0 deletions man/bind_rules.Rd

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

13 changes: 5 additions & 8 deletions man/ruleset_add.Rd

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

69 changes: 69 additions & 0 deletions tests/testthat/test-ruleset_construction.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
test_that("Rules can be added together", {
rule_1 <- rule(mpg >10)
rule_2 <- rule(hp >10)
expect_equal(
rule_1 + rule_2,
Copy link
Owner

Choose a reason for hiding this comment

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

Can you reindent the lines? (If you are using RStudio: CTRL + A then CTRL + I)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing! Assuming we want what's inside the tests indented another time.

ruleset(rule_1, rule_2)
)

})

test_that("Rules can be added to rulesets", {
rule_1 <- rule(mpg >10)
rule_2 <- rule(hp >10)
rule_3 <- rule(name == "henry")

expect_equal(
rule_1 + ruleset(rule_2, rule_3),
ruleset(rule_1, rule_2, rule_3)
)

})

test_that("rulesets can be added to rules", {
rule_1 <- rule(mpg >10)
rule_2 <- rule(hp >10)
rule_3 <- rule(name == "henry")

expect_equal(
ruleset(rule_1, rule_2) + rule_3,
ruleset(rule_1, rule_2, rule_3)
)

})

test_that("rulesets can be added to rules", {
rule_1 <- rule(mpg >10)
rule_2 <- rule(hp >10)
rule_3 <- rule(name == "henry")
rule_4 <- rule(sex == "female")

expect_equal(
ruleset(rule_1, rule_2) + ruleset(rule_3, rule_4),
ruleset(rule_1, rule_2, rule_3, rule_4)
)

})

test_that("bind_rules works", {
rule_1 <- rule(mpg >10)
rule_2 <- rule(hp >10)
rule_3 <- rule(name == "henry")
rule_4 <- rule(sex == "female")

expect_equal(
bind_rules(list(rule_1, rule_2, ruleset(rule_3, rule_4))),
ruleset(rule_1, rule_2, rule_3, rule_4)
)

expect_equal(
bind_rules(list(ruleset(rule_1, rule_2), ruleset(rule_3, rule_4))),
ruleset(rule_1, rule_2, rule_3, rule_4)
)

expect_equal(
bind_rules(list(rule_1, ruleset(rule_2), ruleset(rule_3, rule_4))),
ruleset(rule_1, rule_2, rule_3, rule_4)
)

})