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

data_rename(): "Error: Following variable(s) were not found" in version '0.13.0.17' #571

Closed
rempsyc opened this issue Dec 12, 2024 · 7 comments · Fixed by #572
Closed

data_rename(): "Error: Following variable(s) were not found" in version '0.13.0.17' #571

rempsyc opened this issue Dec 12, 2024 · 7 comments · Fixed by #572
Assignees

Comments

@rempsyc
Copy link
Member

rempsyc commented Dec 12, 2024

Yo, trying to integrate the devel version of datawizard '0.13.0.17' I'm facing a breaking change in data_rename() (more info in easystats/report#470). Just wanted to check with you if that's something I should change in report or datawizard. My attempt at a minimal reprex:

datawizard '0.13.0.17'

packageVersion("datawizard")
#> [1] '0.13.0.17'

result <- list(
  setosa.a = structure(data.frame(1)), 
  versicolor.a = structure(data.frame(2)))

new_names <- c("setosa, a", "versicolor, a")

names(result)
#> [1] "setosa.a"     "versicolor.a"
new_names
#> [1] "setosa, a"     "versicolor, a"

datawizard::data_rename(result, select = names(result), replacement = new_names)
#> Error: Following variable(s) were not found: setosa.a, versicolor.a
#>   Possibly misspelled?

Created on 2024-12-12 with reprex v2.1.1

datawizard '0.13.0'

packageVersion("datawizard")
#> [1] '0.13.0'

result <- list(
  setosa.a = structure(data.frame(1)), 
  versicolor.a = structure(data.frame(2)))

new_names <- c("setosa, a", "versicolor, a")

names(result)
#> [1] "setosa.a"     "versicolor.a"
new_names
#> [1] "setosa, a"     "versicolor, a"

datawizard::data_rename(result, select = names(result), replacement = new_names)
#> $`setosa, a`
#>   X1
#> 1  1
#> 
#> $`versicolor, a`
#>   X2
#> 1  2

Created on 2024-12-12 with reprex v2.1.1

The reason seems to be that data_rename() searches for all the variables mentioned in select for each data frame of the list, but each name only appears once in their respective data frame. Therefore, some variables are bound to not be found. So, whereas data_rename() used to work on lists, now it only works on dataframes? This seems to break report::report_sample tests.

@strengejacke
Copy link
Member

I'm not sure it was really intended to work on lists, so the revision of data_rename() seems to break this feature now. But we could add it back?

@strengejacke
Copy link
Member

Note that you cannot use the safe argument in future versions and have to check if names (in select) really exist.

@etiennebacher
Copy link
Member

I'm not sure it was really intended to work on lists, so the revision of data_rename() seems to break this feature now. But we could add it back?

I don't think we should support lists, other data_ functions also don't:

library(datawizard)

d <- data.frame(a = 1:3, b = 4:6)
l <- list(a = 1:3, b = 4:6)

data_select(d, "a")
#>   a
#> 1 1
#> 2 2
#> 3 3
data_select(l, "a")
#> Warning: No column names that matched the required search pattern were found.
#> NULL

It would also be confusing: should data_rename() modify the names of the list or the names of each element in the list?

@strengejacke
Copy link
Member

I think so, too. I don't think this was actually intentional. You could use setNames() as a workaround in this particular situation, @rempsyc

@etiennebacher
Copy link
Member

etiennebacher commented Dec 13, 2024

Maybe we should change the description of the data argument since it says:

A data frame, or an object that can be coerced to a data frame.

A list is coercible to a dataframe but we don't support it.

@strengejacke
Copy link
Member

Yes, thought about that, too. I think this mostly makes sense for data frames. The alternative would be to revise .select_nse(), to it also works with list. Haven't looked into the details whether this can be done easily.

@rempsyc
Copy link
Member Author

rempsyc commented Dec 13, 2024

Yes, agreed, it would make sense for consistency that it works only on data frames. And, it was only used that way once in report, so I was able to switch it to setNames() like Daniel suggested. Perhaps we should simply have a more informative error on lists, or, as you mention, not attempt to coerce.

strengejacke added a commit that referenced this issue Dec 13, 2024
…sion '0.13.0.17' (#572)

* `data_rename()`: "Error: Following variable(s) were not found" in version '0.13.0.17'
Fixes #571

* version

* comment

* styler, lintr

* docs

* no lists

* Update tests/testthat/test-data_rename.R

Co-authored-by: Etienne Bacher <[email protected]>

* fix docs

* fix

* shorter if-condition

* fix

---------

Co-authored-by: Etienne Bacher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants