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

Introduction vignette #5159

Merged
merged 19 commits into from
Sep 11, 2023
Merged

Introduction vignette #5159

merged 19 commits into from
Sep 11, 2023

Conversation

teunbrand
Copy link
Collaborator

This PR attempts to fix #4620.

Briefly, it adds a vignette that attempts to introduce new people to the essentials of ggplot2 in less than 1000 words.

I'm not an avid writer, so I'd welcome any feedback, including but not limited to spelling fixes, suggestions for phrases, structural comments, rmarkdown faux-pas corrections, general tips etc.

Also, I took @thomasp85's idea for a graphic to help introducing ggplot2.

@teunbrand teunbrand marked this pull request as ready for review February 2, 2023 19:43
@teunbrand teunbrand changed the title WIP: Introduction vignette Introduction vignette Feb 2, 2023
@thomasp85
Copy link
Member

@mine-cetinkaya-rundel if you have time to read this through with the eyes of an educator I'd be grateful. If not that is totally fine

Copy link
Member

@mine-cetinkaya-rundel mine-cetinkaya-rundel 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 contributing this vignette, I can see the value in an introductory vignette in a package. However, I do think a few non-trivial updates would make this vignette much stronger and much more useful to the new learner.

  1. Start with the final graphic and break it down: This is the approach we use in R4DS (https://r4ds.hadley.nz/data-visualize.html#sec-ultimate-goal) and generally my teaching philosophy (cake first!). This gives a direction and purpose from the beginning and then learners are exposed to the pieces that build up to that goal.
  2. Avoid filler words and use accurate terms instead: I've commented in a few places in the vignette about words like "thing", "something", etc. I think an introductory vignette is a great place to expose learners to the terminology that will help them not just learn but also articulate their questions better.
  3. Highlight an example that makes all layers sensible: My suggestion in the review comments is cty and hwy as variables so that coord_fixed() can be sensible, for example.

The main question that @thomasp85 will want to answer though is whether a vignette that replicates the approach of introductory materials from elsewhere (in this case R4DS) is a useful addition or if it's sufficient to link to those resources from the package docs. I can see the value in both ways. If the goal is to include this vignette, I'd be happy to review the revised version.

vignettes/intro.Rmd Outdated Show resolved Hide resolved

## Mapping

The [mapping](https://ggplot2-book.org/getting-started.html#aesthetics) of a plot is a set of instructions on how parts of the data are mapped onto aesthetic attributes of geometric objects. It allows any tidy data to be understood by the graphics system.

Choose a reason for hiding this comment

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

"understood by the graphics system" sounds vague/odd, I think it's more about how the graphics system will use/visualize the data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the last sentence here, does the analogy 'It is the 'dictionary' to translate tidy data to the graphics system.' work well enough?

vignettes/intro.Rmd Outdated Show resolved Hide resolved
vignettes/intro.Rmd Outdated Show resolved Hide resolved
vignettes/intro.Rmd Outdated Show resolved Hide resolved
vignettes/intro.Rmd Outdated Show resolved Hide resolved
vignettes/intro.Rmd Outdated Show resolved Hide resolved
vignettes/intro.Rmd Outdated Show resolved Hide resolved
vignettes/intro.Rmd Outdated Show resolved Hide resolved
@teunbrand
Copy link
Collaborator Author

Thanks for the review, Mine, these comments all contain pieces of wisdom I hadn't yet considered!

My personal thoughts are that this vignette should very tersely show all layers of the grammar involved and serve as advertisement to the ggplot2 book where the information is a lot richer. I'll await Thomas' judgement what, and whether, this vignette should be.

@thomasp85
Copy link
Member

let's go with adding this as a "getting started" vignette (i.e. rename the vignette to ggplot2.rmd) and incorporate the suggestions Mine made

@teunbrand
Copy link
Collaborator Author

I think this ready for further review

Copy link
Member

@mine-cetinkaya-rundel mine-cetinkaya-rundel left a comment

Choose a reason for hiding this comment

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

The revised version looks great. I made a series of comments that I recommend incorporating before merging.

vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
vignettes/ggplot2.Rmd Show resolved Hide resolved
vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
vignettes/ggplot2.Rmd Show resolved Hide resolved
vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
vignettes/ggplot2.Rmd Outdated Show resolved Hide resolved
Co-authored-by: Mine Cetinkaya-Rundel <cetinkaya.mine@gmail.com>
@teunbrand
Copy link
Collaborator Author

Thank you so much @mine-cetinkaya-rundel for reviewing all this, you have a sharp eye for this. I've incorporated your latest suggestions. I didn't add alt-text to some places where you mentioned it was missing, mostly because the plot was hidden in the chunk options.

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

LGTM

@teunbrand teunbrand merged commit f6269b0 into tidyverse:main Sep 11, 2023
12 checks passed
@teunbrand teunbrand deleted the intro_vignette branch September 11, 2023 17:05
@yutannihilation
Copy link
Member

Thanks for this great introduction vignette!

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

Successfully merging this pull request may close these issues.

"Introduction to ggplot2" vignette
4 participants