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

Always use comma notation for subsetting data frame columns #88

Open
jsta opened this issue Sep 23, 2020 · 1 comment
Open

Always use comma notation for subsetting data frame columns #88

jsta opened this issue Sep 23, 2020 · 1 comment
Labels
good first issue Good issue for first-time contributors type:clarification Suggest change for make lesson clearer

Comments

@jsta
Copy link
Member

jsta commented Sep 23, 2020

Lines L315:L325 in Lesson 5 advocate for subsetting columns without comma notation:

head(gapminder[3])

I'd argue that for clarity sake, we should change this to always use comma notation:

head(gapminder[,3]

See #86

@jsta jsta added good first issue Good issue for first-time contributors type:clarification Suggest change for make lesson clearer labels Sep 23, 2020
durandsinclair pushed a commit to durandsinclair/r-intro-geospatial that referenced this issue Dec 20, 2020
@albhasan
Copy link
Contributor

I disagree with this suggestion because the objects returned by each operation are different:

> class(gapminder[3]) == class(gapminder[ ,3])
[1] FALSE
>
> mode(gapminder[3]) == mode(gapminder[ ,3])
[1] FALSE
>
> typeof(gapminder[3]) == typeof(gapminder[ ,3])
[1] FALSE

However, both objects contain the same values:

> all(gapminder[3] == gapminder[ ,3])
[1] TRUE

For beginners, or even experts, this subtle change could go unnoticed and produce unexpected consequences.

For that reason, I think it's better to close this issue without implementing the suggested changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for first-time contributors type:clarification Suggest change for make lesson clearer
Projects
None yet
Development

No branches or pull requests

2 participants