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

rename_with.sf doesn't pass arguments as per dplyr::rename_with help file #2310

Closed
andrewbaxter439 opened this issue Jan 8, 2024 · 6 comments

Comments

@andrewbaxter439
Copy link

As discovered in a SO post, when renaming an sf object with rename_with.sf, it doesn't pass the arguments to the inner function via ... as expected:

library(sf)
#> Linking to GEOS 3.11.2, GDAL 3.6.2, PROJ 9.2.0; sf_use_s2() is TRUE
library(stringr)

nc <- sf::st_read(system.file("shape/nc.shp", package = "sf"), quiet = TRUE)

# This works:
nc |> 
    dplyr:::rename_with.data.frame(str_remove, pattern = "\\d")
#> Simple feature collection with 100 features and 14 fields
#> Geometry type: MULTIPOLYGON
#> Dimension:     XY
#> Bounding box:  xmin: -84.32385 ymin: 33.88199 xmax: -75.45698 ymax: 36.58965
#> Geodetic CRS:  NAD27
#> First 10 features:
#> ...

# `replace_with.sf` method doesn't pass argument in `...`:
nc |> 
    rename_with(str_remove, pattern = "\\d")
#> Error in rename_with(nc, str_remove, pattern = "\\d"): could not find function "rename_with"

Suggested fix (pr to follow) would be to add ... to .fn() call here:

names(agr) = .fn(names(agr))

@edzer
Copy link
Member

edzer commented Jan 20, 2024

I am seeing

> nc |> 
    rename_with(str_remove, pattern = "\\d")
Error in .fn(names(agr)) : argument "pattern" is missing, with no default
> traceback()
6: vctrs::vec_size_common(string = string, pattern = pattern, replacement = replacement, 
       .call = error_call)
5: check_lengths(string, pattern, replacement)
4: str_replace(string, pattern, "")
3: .fn(names(agr))
2: rename_with.sf(nc, str_remove, pattern = "\\d")
1: rename_with(nc, str_remove, pattern = "\\d")

is that the problem you are referring to?

@andrewbaxter439
Copy link
Author

andrewbaxter439 commented Jan 20, 2024

Yes sorry, I'm not sure what happened with my example code above - had assumed it had run the same as that before I posted it!

Edit - I see I forgot to run the library(dplyr) line above my code when making a reprex. That's exactly the error I got.

@edzer edzer closed this as completed in 0740152 Jan 20, 2024
@edzer
Copy link
Member

edzer commented Jan 20, 2024

Thanks!

@andrewbaxter439
Copy link
Author

andrewbaxter439 commented Jan 22, 2024

As a brief question - in #2312 I noted that the names(agr) = .fn(names(agr), ...) doesn't seem to add much functionality and perhaps messes up the names. Is this step truly needed and is the dropping of non-rename_with-transformed names intended/acceptable?

When I added testing to the updated rename_with.sf function it explicitly failed to return the expected object as the "agr" attribute was changed. I do realise I don't know the full depths of what this line accomplishes so do ignore if not relevant.

@edzer
Copy link
Member

edzer commented Jan 22, 2024

The docs of rename_with say ... additional arguments passed onto ‘.fn’ - what could go wrong?

@andrewbaxter439
Copy link
Author

andrewbaxter439 commented Jan 29, 2024

It works for the column names well, but the second step (the names(agr) bit) seems to rename the selected columns in the "agr" attribute but NA the rest:

fn = function(x) paste0(x, "_renamed")

# Before renaming:
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

# After renaming:
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

Had only discovered when trying to write a test which fails:

fn = function(x) paste0(x, "_renamed")

expect_identical(nc %>% rename_with(fn, c(FIPS, FIPSNO)), 
				 nc %>% rename(FIPS_renamed = FIPS, FIPSNO_renamed = FIPSNO))

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