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

Update 07-vector-shapefile-attributes-in-r.Rmd #448

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rfbrown
Copy link

@rfbrown rfbrown commented Jun 24, 2024

Please briefly summarise the changes made in the pull request, and the reason(s) for making these changes.

In the line width example, the size aesthetic doesn't work (i.e., the lines are all the same size). We updated to linewidth as a proposed solution. However, we get the following warning (which also appears when using size), so this may not be the best fix: "Warning message: Using linewidth for a discrete variable is not advised."

In the line width example, the "size" aesthetic doesn't work... updated to "linewidth"
Copy link

🆗 Pre-flight checks passed 😃

This pull request has been checked and contains no modified workflow files, spoofing, or invalid commits.

It should be safe to Approve and Run the workflows that need maintainer approval.

@rfbrown
Copy link
Author

rfbrown commented Jun 24, 2024

Note that this issue shows up again in the challenge, so the fix should be consistently applied throughout the lesson.

@Karel-Kroeze
Copy link

the warning is correct, it doesn't make a lot of sense to use a discrete value (that could be ordered in terms of 'size' of path, but isn't) and map it onto what is by convention if not definition an aesthetic usually used for continuous or at least ordered data.

That said, for illustrative purposes it's fine, and it might even be a good segue into letting learners think a bit about how best to visualize different types of attributes, and letting them know that ggplot2 has a lot of (usually very reasonable) opinions and defaults.

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.

2 participants