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

Remove name renaming #2312

Closed

Conversation

andrewbaxter439
Copy link

An alternative fix which would also close #2310 - these lines actually don't seem to matter with the new dplyr::rename_with functionality:

sf/R/tidyverse.R

Lines 272 to 273 in 99d6565

names(agr) = .fn(names(agr))
st_agr(ret) = agr

They re-run the .fn function in rename_with across all columns. The tests aren't currently picking up that the "agr" attribute is being changed by the inner function:

fn = function(x) paste0(x, "_renamed")
nc %>% attr("agr")
#>      AREA PERIMETER     CNTY_   CNTY_ID      NAME      FIPS    FIPSNO  CRESS_ID 
#>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA> 
#>     BIR74     SID74   NWBIR74     BIR79     SID79   NWBIR79 
#>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA> 
#> Levels: constant aggregate identity
nc %>% rename_with(fn, c(FIPS, FIPSNO)) %>% attr("agr")
#>           <NA>           <NA>           <NA>           <NA>           <NA> 
#>           <NA>           <NA>           <NA>           <NA>           <NA> 
#>   FIPS_renamed FIPSNO_renamed           <NA>           <NA>           <NA> 
#>           <NA>           <NA>           <NA>           <NA>           <NA> 
#>           <NA>           <NA>           <NA>           <NA> 
#>           <NA>           <NA>           <NA>           <NA> 
#> Levels: constant aggregate identity

Removing lines 272-273 works as expected:

nc %>% attr("agr")
#>      AREA PERIMETER     CNTY_   CNTY_ID      NAME      FIPS    FIPSNO  CRESS_ID 
#>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA> 
#>     BIR74     SID74   NWBIR74     BIR79     SID79   NWBIR79 
#>      <NA>      <NA>      <NA>      <NA>      <NA>      <NA> 
#> Levels: constant aggregate identity
nc %>% rename_with(fn, c(FIPS, FIPSNO)) %>% attr("agr")
#>           AREA      PERIMETER          CNTY_        CNTY_ID           NAME 
#>           <NA>           <NA>           <NA>           <NA>           <NA> 
#>   FIPS_renamed FIPSNO_renamed       CRESS_ID          BIR74          SID74 
#>           <NA>           <NA>           <NA>           <NA>           <NA> 
#>        NWBIR74          BIR79          SID79        NWBIR79 
#>           <NA>           <NA>           <NA>           <NA> 
#> Levels: constant aggregate identity

Added new test for arguments to rename_with and altered test for previous update to more robustly test that the renaming function is applied properly. Would recommend this over #2311

@andrewbaxter439
Copy link
Author

Issue #2310 fixed in pr 0740152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rename_with.sf doesn't pass arguments as per dplyr::rename_with help file
1 participant