Skip to content

Commit

Permalink
Fix serious bug with '%to%' in the multiassignment expression in the …
Browse files Browse the repository at this point in the history
…'let'
  • Loading branch information
gdemin committed May 24, 2021
1 parent 5b857b2 commit e34f234
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 3 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: maditr
Type: Package
Title: Fast Data Aggregation, Modification, and Filtering with Pipes and 'data.table'
Version: 0.8.1
Version: 0.8.2
Maintainer: Gregory Demin <gdemin@gmail.com>
Authors@R: person("Gregory", "Demin", email = "gdemin@gmail.com", role = c("aut", "cre"))
Depends: R (>= 3.3.0)
Expand Down
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
0.8.2 (2021-05-25)
================
* fix serious bug with '%to%' in the multiassignment expression in the 'let'

0.8.1 (2021-05-18)
================
* new function 'rows' for selecting rows/filtering dataset
Expand Down
14 changes: 12 additions & 2 deletions R/columns.R
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ select_columns = function(..., data_names, frame, type){
))
all_indexes = create_list_with_index(data_names)
var_list = eval(var_list, all_indexes, frame)
# here we processed values which didn't resolved to numeric column index
var_indexes = expand_characters(var_list, data_names, frame)
var_indexes = unique(unlist(var_indexes, recursive = TRUE, use.names = FALSE))
switch(type,
Expand Down Expand Up @@ -176,6 +177,7 @@ create_columns = function(..., data_names, frame, type){
}

eval_expressions = function(var_list, frame){
# eval expressions, symbols remain as is
if(length(var_list)>1){
var_list = as.list(var_list)
var_list[-1] = lapply(var_list[-1], function(item){
Expand Down Expand Up @@ -252,15 +254,23 @@ create_range_expander = function(df_names, frame, new = FALSE){
if(is.call(to)) to = eval(to, frame)
if(is.symbol(from)) from = as.character(from)
if(is.symbol(to)) to = as.character(to)
if(is.numeric(from) && is.numeric(to) && !new) return(from:to)
if(is.numeric(from) && is.numeric(to)) { #
if(new){
res = df_names[from:to]
!anyNA(res) || stop("'columns' range selection: positions ", from, ":", to, " out of range.")
return(res)
} else {
return(from:to)
}
}
first = match(from, df_names)[1]
last = match(to, df_names)[1]
if(is.na(first) && is.na(last) && new) return(
create_name_sequence(from, to)
)
(!is.na(first) && !is.na(last)) || stop("'columns' range selection: variables '", from, "' or '", to, "' are not found.")
(last>=first) || stop( "'columns' range selection: '",to, "' located before '",from,"'. Did you mean '",to," %to% ",from,"'?")
return(first:last)
if(new) df_names[first:last] else first:last
}
}

Expand Down
67 changes: 67 additions & 0 deletions inst/tinytest/test_selectors.R
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,73 @@ expect_identical(
dt_mt2[, c("a1", "a2", "a3") := list(1,2,3)]
)

dt_mt = as.data.table(mtcars)
dt_mt2 = data.table::copy(dt_mt)

expect_identical(
let(dt_mt,
vs %to% am := 42
),
dt_mt2[, c("vs", "am") := 42]
)

dt_mt = as.data.table(mtcars)
dt_mt2 = data.table::copy(dt_mt)


expect_identical(
let(dt_mt,
c("hp", vs %to% am) := 42
),
dt_mt2[, c("hp", "vs", "am") := 42]
)

dt_mt = as.data.table(mtcars)
dt_mt2 = data.table::copy(dt_mt)


expect_identical(
let(dt_mt,
cols(hp, vs %to% am) := 42
),
dt_mt2[, c("hp", "vs", "am") := 42]
)

dt_mt = as.data.table(mtcars)
dt_mt2 = data.table::copy(dt_mt)


expect_identical(
let(dt_mt,
cols(new_col, vs %to% am) := list(1,2,3)
),
dt_mt2[, c("new_col", "vs", "am") := list(1,2,3)]
)

dt_mt = as.data.table(mtcars)
dt_mt2 = data.table::copy(dt_mt)


expect_identical(
let(dt_mt,
cols(vs1 %to% vs3, new_col) := list(1,2,3, 4)
),
dt_mt2[, c("vs1", "vs2", "vs3", "new_col") := list(1,2,3, 4)]
)

dt_mt = as.data.table(mtcars)
dt_mt2 = data.table::copy(dt_mt)

expect_error(
let(dt_mt, 12 %to% 15 := 42)
)

expect_identical(
let(dt_mt,
cols(1 %to% 3, new_col) := list(1,2,3, 4)
),
dt_mt2[, (1:3) := list(1,2,3)][,new_col:=4]
)

# dt_mt = as.data.table(mtcars)
# dt_mt2 = data.table::copy(dt_mt)
Expand Down

0 comments on commit e34f234

Please sign in to comment.