Skip to content

Use {palmerpenguins} dataset in arrange() examples #7676

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

KittJonathan
Copy link

I added examples using the penguins dataset in the arrange() documentation

  • added {palmerpenguins} to suggests in the dplyr DESCRIPTION file

@DavisVaughan
Copy link
Member

We really appreciate the PR! I do not currently see any reason to switch away from mtcars to penguins in this example. The story is a little complicated since R >=4.5 already has penguins built in, meaning we shadow it by loading this package and users would get a possibly confusing message

Screenshot 2025-04-14 at 8 46 36 AM

I don't think switching to penguins right now is worth the mental overhead of having to remember that there are now 2 datasets with the same name. Also, the column names of both are unfortunately different, making it even more confusing.

I do however like your additional inline comments, and how you added an extra example of arranging by just 1 column. I would accept a PR that kept mtcars but added those inline comments and the extra example, if you'd like to do that.

@yutannihilation
Copy link
Member

I do not currently see any reason to switch away from mtcars to penguins in this example

A bit drifted topic: On the other hand, I believe there are reasons for dplyr (and other tidyverse packages) to switch from iris to penguins because I believe that's the very reason penguins was added.

ggplot2 removed iris example code 5 years ago. I was hoping other packages would follow, but it had not happen so far. I think it's a good time to consider migrating to penguins.

That said, I have no idea how the replacement should be done. basepenguins might help, but I'm not fully sure if it won't make another confusion.

@KittJonathan
Copy link
Author

I get your point @DavisVaughan. I can indeed work on using mtcars. But I personally do think it would be nice having examples using palmerpenguins across as many tidyverse functions as possible, given the popularity of the dataset in tutorials and courses.
@hadley clarified by email the tidyverse version policy, supporting the current and previous four versions of R.
Would it be worth while considering switching to palmerpenguins (or basepenguins) examples when four versions of R have been released?

@hadley
Copy link
Member

hadley commented Apr 15, 2025

I think this change is worth it, but you need to (a) put palmerpenguins in imports, not suggests (since otherwise you need to protect all the examples) and (b) load the data using data() instead of by attaching the package. I'd also suggest keeping the existing pipe based examples primary, since I don't think we want to get into that discussion now.

I can't believe that R core changed the variable names, making the whole substitution more complicated.

KittJonathan and others added 2 commits April 15, 2025 16:11
Co-authored-by: Davis Vaughan <[email protected]>
Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for continuing to work on this! It's really important to get the details 100% correct in this prototype, so I appreciate you bearing with us as we deliver very detailed feedback.

@KittJonathan
Copy link
Author

Thanks for continuing to work on this! It's really important to get the details 100% correct in this prototype, so I appreciate you bearing with us as we deliver very detailed feedback.

Thanks for your help, I appreciate you taking the time to give very detailed feedback :-)

KittJonathan and others added 3 commits April 16, 2025 15:25
Co-authored-by: Hadley Wickham <[email protected]>
- update DESCRIPTION to preserver alphabetical order
- remove unrelated file from .gitignore
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.

4 participants