Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

`rename()` produces error/warning/message/silent of duplicate names would result -addresses #127 #186

Closed
wants to merge 69 commits into from

7 participants

@wibeasley

The test suite sometimes gives errors like

rbind.fill performance linear with factors -
p[2]/p[1] < threshold isn't true.

But I'm pretty sure it's unrelated to changes I've made to plyr::rename().

and others added some commits
@hadley Add CITATION file f28c5e7
@hadley Add references to help files bf183b1
@hadley Update news b8a9f1b
@hadley Bump version ff21040
@hadley Version bump 14e03e2
@hadley Only copy dims in alply if asked 0f56d5c
@hadley Fix dumb typo in d_ply. Fixes #112 fce1d86
@hadley alply .dims should default to FALSE, of course e501123
@hadley Specific .dims = TRUE in test 0898aaf
@hadley Only run parallel tests is doMC available eb35085
@hadley Use R vector instead of VLA.
Fix windows segfault?
2f74318
@wch wch Fix partial argument matches aa82a6e
@wch wch Revert C code changes from 03e1af8 because they don't fix the problem d03400b
@hadley Drop empty inputs to id ae5b073
@hadley Fix a couple of small doc problems. Reported by Henrik Parn 934d508
@hadley Better array allocation in split_indices
Fixes #131
e571d77
@krlmlr krlmlr ignore RStudio files fd646d7
@krlmlr krlmlr change case of extension for files in man-roxygen; fixes #141 6337996
@tjmahr tjmahr Update test-mutate.r 68a2d81
@richierocks richierocks Update count.r
Changed test from `is.vector` to `is.atomic`, in order to fix Issue 130.
4ff1827
@richierocks richierocks Update test-count.r
Added tests for passing factors and Dates to count.
23263e0
@richierocks richierocks Bug fix to #130 9c80d8d
@wch wch mapvalues: use warn_missing for factors. Fixes #180 f27c9e2
@wch wch Bump version to 1.8.0.99 for development b8053fe
@wch wch Re-document with roxygen2 3.0.0 d1858cd
@wch wch Add tests for warn_missing with revalue and mapvalues 4712104
@hadley Convert to Rcpp 1ba61eb
@hadley Update docs 33973f9
@hadley Export loop_apply af4e6cc
@hadley Add rpoj file a915219
@hadley Add travis support fbcc50c
@hadley Install testthat from github: f10b0db
@hadley Use a couple of binary packages ba11357
@hadley Don't build travis file 43b9b5e
@hadley Fix bug in id_var when factor levels missing 13d22d5
@hadley Ignore rhistory 086393a
@hadley Fix another problem with missing levels 8847d73
@hadley Add build status to readme 2ebade0
@hadley Revert to older testthat 9f66fd2
@hadley Don't export loop_apply 1da35c4
@hadley Update documentation e794c76
@hadley Fix link to '.'. Closes #125 1c452bc
@hadley Don't use loop_apply examples 84cc895
@hadley Don't drop missing values when sorting.
Closes #169
8956d85
@crowding crowding rbind.fill checks for factor-on-character. Fixes #128 b28ee12
@crowding crowding update NEWS 3999f7f
@crowding crowding output_template treats dims orthogonal to type d520c0d
@crowding crowding extract method 'allocate_column' 0a349d5
@crowding crowding rbind.fill accumulates data in closures 89e7656
@crowding crowding Compute attrs before alloc; specialize mutators
This acheives linear performance for rbind.fill on arrays, chars, lists.

Factors and POSIXt values will still be quadratic.
b5c03fd
@crowding crowding rbind.fill multiway arrays 4c72944
@crowding crowding rbind.fill linear tests; linear factors f7e32ba
@crowding crowding Handle POSIXct in linear time 1d7c317
@crowding crowding Format and update NEWS c3dd711
@crowding crowding rbind.fill respects matrix column dimnames 97a5a2e
@crowding crowding Always delay setting class attribute aa7344d
@crowding crowding Delete unused function 47317ac
@crowding crowding Adjust comments 247142b
@crowding crowding Clean up test code 424ca86
@crowding crowding Organization suggestions db3d115
@crowding crowding Check for data frame columns 487ce32
@crowding crowding strip dimnames for 1d arrays 29a0d9a
@crowding crowding rbind.fill docs 19b7db8
@wibeasley wibeasley Notify if duplicate names exist after `rename()` 1ee5e92
@wibeasley wibeasley Merge remote-tracking branch 'upstream/master'
Conflicts:
	NEWS
	inst/tests/test-rbind.r
14fd290
@wibeasley wibeasley Empty Commit 7de2985
@wibeasley wibeasley Gives error/warning/message/silent if renames are duplicated.
The test suite sometimes gives errors like "rbind.fill performance linear with factors -
p[2]/p[1] < threshold isn't true".  But I think it's unrelated to changes I've made to `plyr::rename()`.
24cda00
@wibeasley wibeasley regexp used for expected error/warning/messages b3a0da8
@wibeasley wibeasley Added a test for double swapping. Also commented the test code a little. ba04eec
@hadley
Owner

Could you please rebase/merge off master to avoid all the extra commits?

@wibeasley

I'm closing this pull request in favor of the more concise #194

@wibeasley wibeasley closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 5, 2011
  1. Add CITATION file

    authored
  2. Add references to help files

    authored
  3. Update news

    authored
  4. Bump version

    authored
Commits on Jan 3, 2014
  1. @wibeasley

    Version bump

    authored wibeasley committed
  2. @wibeasley

    Only copy dims in alply if asked

    authored wibeasley committed
  3. @wibeasley

    Fix dumb typo in d_ply. Fixes #112

    authored wibeasley committed
  4. @wibeasley

    alply .dims should default to FALSE, of course

    authored wibeasley committed
  5. @wibeasley

    Specific .dims = TRUE in test

    authored wibeasley committed
  6. @wibeasley

    Only run parallel tests is doMC available

    authored wibeasley committed
  7. @wibeasley

    Use R vector instead of VLA.

    authored wibeasley committed
    Fix windows segfault?
  8. @wch @wibeasley

    Fix partial argument matches

    wch authored wibeasley committed
  9. @wch @wibeasley
  10. @wibeasley

    Drop empty inputs to id

    authored wibeasley committed
  11. @wibeasley
  12. @wibeasley

    Better array allocation in split_indices

    authored wibeasley committed
    Fixes #131
  13. @krlmlr @wibeasley

    ignore RStudio files

    krlmlr authored wibeasley committed
  14. @krlmlr @wibeasley
  15. @tjmahr @wibeasley

    Update test-mutate.r

    tjmahr authored wibeasley committed
  16. @richierocks @wibeasley

    Update count.r

    richierocks authored wibeasley committed
    Changed test from `is.vector` to `is.atomic`, in order to fix Issue 130.
  17. @richierocks @wibeasley

    Update test-count.r

    richierocks authored wibeasley committed
    Added tests for passing factors and Dates to count.
  18. @richierocks @wibeasley

    Bug fix to #130

    richierocks authored wibeasley committed
  19. @wch @wibeasley

    mapvalues: use warn_missing for factors. Fixes #180

    wch authored wibeasley committed
  20. @wch @wibeasley

    Bump version to 1.8.0.99 for development

    wch authored wibeasley committed
  21. @wch @wibeasley

    Re-document with roxygen2 3.0.0

    wch authored wibeasley committed
  22. @wch @wibeasley

    Add tests for warn_missing with revalue and mapvalues

    wch authored wibeasley committed
  23. @wibeasley

    Convert to Rcpp

    authored wibeasley committed
  24. @wibeasley

    Update docs

    authored wibeasley committed
  25. @wibeasley

    Export loop_apply

    authored wibeasley committed
  26. @wibeasley

    Add rpoj file

    authored wibeasley committed
  27. @wibeasley

    Add travis support

    authored wibeasley committed
  28. @wibeasley

    Install testthat from github:

    authored wibeasley committed
  29. @wibeasley

    Use a couple of binary packages

    authored wibeasley committed
  30. @wibeasley

    Don't build travis file

    authored wibeasley committed
  31. @wibeasley

    Fix bug in id_var when factor levels missing

    authored wibeasley committed
  32. @wibeasley

    Ignore rhistory

    authored wibeasley committed
  33. @wibeasley

    Fix another problem with missing levels

    authored wibeasley committed
  34. @wibeasley

    Add build status to readme

    authored wibeasley committed
  35. @wibeasley

    Revert to older testthat

    authored wibeasley committed
  36. @wibeasley

    Don't export loop_apply

    authored wibeasley committed
  37. @wibeasley

    Update documentation

    authored wibeasley committed
  38. @wibeasley

    Fix link to '.'. Closes #125

    authored wibeasley committed
  39. @wibeasley

    Don't use loop_apply examples

    authored wibeasley committed
  40. @wibeasley

    Don't drop missing values when sorting.

    authored wibeasley committed
    Closes #169
  41. @crowding @wibeasley

    rbind.fill checks for factor-on-character. Fixes #128

    crowding authored wibeasley committed
  42. @crowding @wibeasley

    update NEWS

    crowding authored wibeasley committed
  43. @crowding @wibeasley

    output_template treats dims orthogonal to type

    crowding authored wibeasley committed
  44. @crowding @wibeasley

    extract method 'allocate_column'

    crowding authored wibeasley committed
  45. @crowding @wibeasley

    rbind.fill accumulates data in closures

    crowding authored wibeasley committed
  46. @crowding @wibeasley

    Compute attrs before alloc; specialize mutators

    crowding authored wibeasley committed
    This acheives linear performance for rbind.fill on arrays, chars, lists.
    
    Factors and POSIXt values will still be quadratic.
  47. @crowding @wibeasley

    rbind.fill multiway arrays

    crowding authored wibeasley committed
  48. @crowding @wibeasley

    rbind.fill linear tests; linear factors

    crowding authored wibeasley committed
  49. @crowding @wibeasley

    Handle POSIXct in linear time

    crowding authored wibeasley committed
  50. @crowding @wibeasley

    Format and update NEWS

    crowding authored wibeasley committed
  51. @crowding @wibeasley

    rbind.fill respects matrix column dimnames

    crowding authored wibeasley committed
  52. @crowding @wibeasley

    Always delay setting class attribute

    crowding authored wibeasley committed
  53. @crowding @wibeasley

    Delete unused function

    crowding authored wibeasley committed
  54. @crowding @wibeasley

    Adjust comments

    crowding authored wibeasley committed
  55. @crowding @wibeasley

    Clean up test code

    crowding authored wibeasley committed
  56. @crowding @wibeasley

    Organization suggestions

    crowding authored wibeasley committed
  57. @crowding @wibeasley

    Check for data frame columns

    crowding authored wibeasley committed
  58. @crowding @wibeasley

    strip dimnames for 1d arrays

    crowding authored wibeasley committed
  59. @crowding @wibeasley

    rbind.fill docs

    crowding authored wibeasley committed
  60. @wibeasley
  61. @wibeasley

    Merge remote-tracking branch 'upstream/master'

    wibeasley authored
    Conflicts:
    	NEWS
    	inst/tests/test-rbind.r
  62. @wibeasley

    Empty Commit

    wibeasley authored
Commits on Jan 4, 2014
  1. @wibeasley

    Gives error/warning/message/silent if renames are duplicated.

    wibeasley authored
    The test suite sometimes gives errors like "rbind.fill performance linear with factors -
    p[2]/p[1] < threshold isn't true".  But I think it's unrelated to changes I've made to `plyr::rename()`.
  2. @wibeasley
  3. @wibeasley
This page is out of date. Refresh to see the latest.
View
24 R/rename.r
@@ -5,6 +5,9 @@
#' old names as names.
#' @param warn_missing print a message if any of the old names are
#' not actually present in \code{x}.
+#' @param duplicate_behavior specifies the behavior if duplicates names would result in \code{x}.
+#' Options are \code{error}, \code{warning}, \code{message}, and \code{silent}.
+#' The default is \code{warning}.
#' Note: x is not altered: To save the result, you need to copy the returned
#' data into a variable.
#' @export
@@ -16,7 +19,22 @@
#' x
#' # Rename column "disp" to "displacement"
#' rename(mtcars, c("disp" = "displacement"))
-rename <- function(x, replace, warn_missing = TRUE) {
- names(x) <- revalue(names(x), replace, warn_missing = warn_missing)
- x
+rename <- function(x, replace, warn_missing = TRUE, duplicate_behavior = "warning" ) {
+
+ # This line does the real work of `rename()`.
+ base::names(x) <- plyr::revalue(base::names(x), replace, warn_missing = warn_missing)
+
+ # Check if any names are duplicated
+ tabled_values <- base::table(base::names(x))
+ duplicated_names <- base::names(tabled_values[tabled_values>1])
+ if( base::length(duplicated_names) > 0L ) {
+# response_message <- base::paste0("The plyr::rename operation has created duplicates for the following names: `", base::paste(duplicated_names, collapse="`, `"), "`")
+ response_message <- base::paste0("The plyr::rename operation has created duplicates for the following name(s): (`", base::paste(duplicated_names, collapse="`, `"), "`)")
+ if( duplicate_behavior == "error") base::stop(response_message)
+ else if( duplicate_behavior == "warning" ) base::warning(response_message)
+ else if( duplicate_behavior == "message" ) base::message(response_message)
+ else if( duplicate_behavior != "silent" ) base::stop("The plyr::rename value passed to the `duplicate_behavior` parameter was not recognized.")
+ }
+
+ return( x )
}
View
7 inst/tests/test-rbind.r
@@ -292,10 +292,3 @@ test_that("rbind.fill performance linear with times", {
classes=c("time"))
expect_linear_enough(timings)
})
-
-test_that("NULLs silently dropped", {
- expect_equal(rbind.fill(mtcars, NULL), mtcars)
- expect_equal(rbind.fill(NULL, mtcars), mtcars)
- expect_equal(rbind.fill(NULL, NULL), NULL)
-
-})
View
97 inst/tests/test-rename.r
@@ -1,9 +1,12 @@
-context("Rename")
+#################################################
+### Main-stream cases
+#################################################
+context("Rename - Expected Usage")
test_that("No match leaves names unchanged", {
x <- c(a = 1, b = 2, c = 3, 4)
y <- rename(x, c(d = "e"), warn_missing = FALSE)
-
+
expect_equal(names(x), names(y))
})
@@ -16,14 +19,14 @@ test_that("Missing old values result in message", {
test_that("Single name match makes change", {
x <- c(a = 1, b = 2)
y <- rename(x, c(b = "c"))
-
+
expect_equal(names(y), c("a", "c"))
})
test_that("Multiple names correctly changed", {
x <- c(a = 1, b = 2, c = 3)
y <- rename(x, c("c" = "f", "b" = "e", "a" = "d"))
-
+
expect_equal(names(y), c("d", "e", "f"))
})
@@ -37,3 +40,89 @@ test_that("Renaming lists", {
y <- rename(x, c("c" = "f", "b" = "e", "a" = "d"))
expect_identical(y, list(d = 1, e = 2, f = 3))
})
+
+#################################################
+### Duplicate Names
+#################################################
+context("Rename - Duplicates")
+
+##
+## This batch tests the typical renaming scenarios
+##
+test_that("Renaming list with an conflicting variable name - default", {
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("c" = "f", "b" = "e", "a" = "f")
+ expected_response <- "The plyr::rename operation has created duplicates for the following name\\(s\\): \\(`f`\\)"
+ expect_warning(object = rename(x=x, replace=replace_list), regexp=expected_response)
+})
+test_that("Renaming list with an conflicting variable name - error", {
+ duplicate_behavior <- "error"
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("c" = "f", "b" = "e", "a" = "f")
+ expected_response <- "The plyr::rename operation has created duplicates for the following name\\(s\\): \\(`f`\\)"
+ expect_error(object = rename(x=x, replace=replace_list, duplicate_behavior=duplicate_behavior), regexp=expected_response)
+})
+test_that("Renaming list with an conflicting variable name - warning", {
+ duplicate_behavior <- "warning"
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("c" = "f", "b" = "e", "a" = "f")
+ expected_response <- "The plyr::rename operation has created duplicates for the following name\\(s\\): \\(`f`\\)"
+ expect_warning(object = rename(x=x, replace=replace_list, duplicate_behavior=duplicate_behavior), regexp=expected_response)
+})
+test_that("Renaming list with an conflicting variable name - message", {
+ duplicate_behavior <- "message"
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("c" = "f", "b" = "e", "a" = "f")
+ expected_response <- "The plyr::rename operation has created duplicates for the following name\\(s\\): \\(`f`\\)"
+ expect_message(object = rename(x=x, replace=replace_list, duplicate_behavior=duplicate_behavior), regexp=expected_response)
+})
+test_that("Renaming list with an conflicting variable name - silent", {
+ duplicate_behavior <- "silent"
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("c" = "f", "b" = "e", "a" = "f")
+ expected_value <- list(f = 1, e = 2, f = 3)
+ expect_identical(rename(x=x, replace=replace_list, duplicate_behavior=duplicate_behavior), expected=expected_value)
+})
+
+
+##
+## This batch tests the boundary cases
+##
+test_that("Renaming to the same value", {
+ #One element is renamed to itself
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("a" = "a")
+ expected_value <- x
+ expect_identical(rename(x=x, replace=replace_list), expected=expected_value)
+})
+test_that("Renaming list with an empty renaming vector", {
+ #No renames are requested (which could happen if the calling code was under a lot of automated code.)
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c()
+ expected_value <- x
+ expect_identical(rename(x=x, replace=replace_list), expected=expected_value)
+})
+test_that("Single Swapping (shouldn't cause problems)", {
+ #Notice how a becomes c, while c becomes f.
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("c" = "f", "b" = "e", "a" = "c")
+ expected_value <- list(c = 1, e = 2, f = 3)
+ actual_value <- rename(x=x, replace=replace_list)
+ expect_identical(actual_value, expected=expected_value)
+})
+test_that("Double Swapping (shouldn't cause problems)", {
+ #Notice how a becomes c, while c becomes a.
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("c" = "a", "b" = "z", "a" = "c")
+ expected_value <- list(c = 1, z = 2, a = 3)
+ actual_value <- rename(x=x, replace=replace_list)
+ expect_identical(actual_value, expected=expected_value)
+})
+test_that("Multiple assignments for the same element", {
+ #Notice how it requests to change a to d, e, and f.
+ x <- list(a = 1, b = 2, c = 3)
+ replace_list <- c("a" = "d", "a" = "e", "a" = "f")
+ expected_response <- "The following `from` values were not present in `x`: a, a"
+ expected_value <- list(a = 1, a = 2, a = 3)
+ expect_message(rename(x=x, replace=replace_list), regexp=expected_response)
+})
View
9 man/rename.Rd
@@ -2,7 +2,7 @@
\alias{rename}
\title{Modify names by name, not position.}
\usage{
-rename(x, replace, warn_missing = TRUE)
+rename(x, replace, warn_missing = TRUE, duplicate_behavior = "warning")
}
\arguments{
\item{x}{named object to modify}
@@ -11,7 +11,12 @@ rename(x, replace, warn_missing = TRUE)
values, and old names as names.}
\item{warn_missing}{print a message if any of the old
- names are not actually present in \code{x}. Note: x is
+ names are not actually present in \code{x}.}
+
+ \item{duplicate_behavior}{specifies the behavior if
+ duplicates names would result in \code{x}. Options are
+ \code{error}, \code{warning}, \code{message}, and
+ \code{silent}. The default is \code{warning}. Note: x is
not altered: To save the result, you need to copy the
returned data into a variable.}
}
View
BIN  src/plyr.dll
Binary file not shown
Something went wrong with that request. Please try again.