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

Deprecate .data in data-expressions #169

Closed
lionel- opened this issue Jan 27, 2020 · 15 comments · Fixed by #295
Closed

Deprecate .data in data-expressions #169

lionel- opened this issue Jan 27, 2020 · 15 comments · Fixed by #295
Labels
Milestone

Comments

@lionel-
Copy link
Member

lionel- commented Jan 27, 2020

To make programming with data-masking and programming with selections completely distinct.

@lionel-
Copy link
Member Author

lionel- commented Jan 27, 2020

Do we need an all_of() variant that expects exactly 1 variable?

@dblodgett-usgs
Copy link

Out of curiosity, was the fact that this pattern has been recommended in this vignette https://dplyr.tidyverse.org/articles/programming.html for quite some time taken into account when this change was made? It seems like it's causing a lot of people to go changing a lot of code ... is it really needed?

@hadley
Copy link
Member

hadley commented Oct 16, 2022

@dblodgett-usgs yes

@dblodgett-usgs
Copy link

dblodgett-usgs commented Oct 17, 2022

Well ok then, thanks I guess. It appears the documentation has actually changed at that vignette?

I see you committed a change ~20 days ago but it only deployed to github pages two days ago. Reading that now, it does make more sense... it just seemed strange because I checked that vignette and it hadn't changed.

@chrarnold
Copy link

chrarnold commented Dec 8, 2022

I'd like to know what the best way is regarding this change when it comes to notes during R CMD check? I recently added .data$ to eliminate the (many, many) notes during package checking, but with the newest package versions, look like I need to revert back. Couldnt find it anywhere or I oversaw? Thanks!

@hadley
Copy link
Member

hadley commented Dec 8, 2022

The warning message should tell you what to do:

out <- mtcars |> dplyr::select(.data$cyl)
#> Warning: Use of .data in tidyselect expressions was deprecated in tidyselect 1.2.0.
#> ℹ Please use `"cyl"` instead of `.data$cyl`

Created on 2022-12-08 with reprex v2.0.2

@chrarnold
Copy link

Sorry maybe I was not clear, the warning message is absolutely clear. However, when building packages, R CMD check prints the almighty famous "no visible binding for global variable" note for each instance when not using the .data$ prefix. Before this change, I found the .data$ construct to be the best to avoid this note, I didnt like defining dozens of global variables. Is there any updated recommendation on this in light of this change?

@dblodgett-usgs
Copy link

@chrarnold -- this article was updated recently and has a pretty thorough summary of the details on this. https://dplyr.tidyverse.org/articles/programming.html#data-masking In short, use .data$foo for things that do not use tidyselect and "foo" for things that do use tidyselect. Which function is throwing the error should be a guide but for me it was basically select/rename use "foo" and filter/mutate/group_by use .data$foo. See DOI-USGS/nhdplusTools#306 for my clean up issue on this if it helps.

@lionel-
Copy link
Member Author

lionel- commented Dec 9, 2022

@chrarnold If you use dplyr::select("cyl") (with quotes) instead of dplyr::select(.data$cyl), there will be no warnings.

@chrarnold
Copy link

Thanks @lionel- and @dblodgett-usgs , I didnt know dplyr::select("cyl") can be used, does this work only for newer versions of dplyr or this was already supported for older versions? For the package, I have to find an implementation that is as generically working as possible without having to require particular package versions etc if possible.

@dblodgett-usgs In a tidyselect context, wouldnt the R CMD check notes still be there then? I really want to get rid of these notes, having hundreds of them bloats up the output so much

@lionel-
Copy link
Member Author

lionel- commented Dec 9, 2022

This was also supported in old versions of dplyr.

@dblodgett-usgs In a tidyselect context, wouldnt the R CMD check notes still be there then?

No because in tidyselect contexts you now use quoted names like "foo" as @dblodgett-usgs mentioned.

@chrarnold
Copy link

Thanks, I have not read the "new" quoted recommendation anywhere else to be honest, also not in the quoted (and updated) https://dplyr.tidyverse.org/articles/programming.html#data-masking article. I'll update my code correspondingly!

@lionel-
Copy link
Member Author

lionel- commented Dec 12, 2022

@chrarnold To be clear, in data-masking (as in the link you just shared), a string represents a string, not a column. So you can't do things like 1 + "cyl" because you'd be adding a number to a string.

It's only for column selection that you can refer to a column with a string.

@shahronak47
Copy link

@lionel- Then what is the recommended way to replace .data in mutate ? Since .data is deprecated we can't use mtcars %>% mutate(new = 1 + .data$cyl) or mtcars %>% mutate(new = 1 + .data[["cyl"]])

@lionel-
Copy link
Member Author

lionel- commented Feb 6, 2023

.data is not deprecated in mutate().

It's only been deprecated in tidy selections, e.g. in contexts like select(...) or pivot_longer(cols = ).

damianooldoni added a commit to trias-project/trias that referenced this issue Feb 7, 2023
Based on new version of tidyselect, see r-lib/tidyselect#169
jessesadler added a commit to jessesadler/debkeepr that referenced this issue Feb 10, 2023
* Remove use of .data$account_id in selection and arranging of data frames in transaction function.
* Use data masking in `dplyr::arrange()` and so change to `dplyr::arrange(.data[["account_id"]])`. This eliminates global variable warnings and avoids `arrange()` not working with quoted variable.
* Use quoted variable of "account_id" with `dplyr::select()`.
* Move use of `arrange(account_id)` to end of function so it is only written once, not for each possible outcome.
* Fixes tidyselect issue 169: r-lib/tidyselect#169
nbc added a commit to MTES-MCT/didoscalim that referenced this issue Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants