jkosie / lab1

EDLD610 - Lab 1 Repo
MIT License
0 stars 0 forks source link

Feedback #1

Open datalorax opened 5 years ago

datalorax commented 5 years ago

Thank all!

So you have two .Rproj files in the root directory here. You definitely will only ever want to have one .Rproj for each project. Otherwise you can run into working directory issues. It's not so bad if they're both at the same directory level, but generally it's best avoided anyway.

In terms of your code:

Not a big deal, and sort of just a style preference, but I would recommend adding set class = "tbl_df" as an additional argument to import so it imports as a tibble, rather than a standard data frame. I just think they're a lot nicer to work with.

For interactive work it's generally not that big of a deal, but it's best to avoid using T and F in place of TRUE and FALSE. See here for an example of why: https://www.r-bloggers.com/r-tip-avoid-using-t-and-f-as-synonyms-for-true-and-false/

As is, your area is actually overlaid on top of your line/smooth. I would actually put this as the base layer and put the line/smooth on top of that. You're also missing a few other subtleties like a caption but overall it looks good.

The biggest problem I'm seeing at this point is no Commit 2/3, but I'll come back and look again later (still isn't due yet, after all).

jkosie commented 5 years ago

Thanks!

I just made some edits:

However, when I look at the commits I do see 2 and 3. I'm not sure what needs to be fixed there.

datalorax commented 5 years ago

Yeah I’m not sure but they are not showing up in the Rmd (or, obviously, the HTML). I’m wondering if has to do with the files not being merged correctly (or at all).

Thanks,

Daniel Anderson Research Assistant Professor Behavioral Research and Teaching University of Oregon

On Jan 10, 2019, at 3:21 PM, Jessica Kosie notifications@github.com wrote:

Thanks!

I just made some edits:

added set class = "tbl_df" changed T/F to TRUE/FALSE put area as base layer and line/smooth on top; added caption However, when I look at the commits I do see 2 and 3. I'm not sure what needs to be fixed there.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jkosie/lab1/issues/1#issuecomment-453297585, or mute the thread https://github.com/notifications/unsubscribe-auth/AKb-iBUuu72QwGq_xc27kKRLrAS9D4hFks5vB8sAgaJpZM4Z6eOZ.

jkosie commented 5 years ago

Maybe it had something to do with the extra R project? I didn't look before deleting that project, but (at least now) I see the commits in the Rmd, and they're there when I knit to html.

datalorax commented 5 years ago

Perfect! I’ll look at those commits now.

Thanks,

Daniel Anderson Research Assistant Professor Behavioral Research and Teaching University of Oregon

On Jan 10, 2019, at 3:31 PM, Jessica Kosie notifications@github.com wrote:

Maybe it had something to do with the extra R project? I didn't look before deleting that project, but (at least now) I see the commits in the Rmd, and they're there when I knit to html.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jkosie/lab1/issues/1#issuecomment-453300825, or mute the thread https://github.com/notifications/unsubscribe-auth/AKb-iJvSfRFZQfzCspmEwVQuPfMSCT4Nks5vB81NgaJpZM4Z6eOZ.

datalorax commented 5 years ago

Okay, so here's the rest of your feedback:

I see you're loading the utf8 library... is there a reason that's needed? Generally I recommend against loading packages that aren't needed for the specific script because it can lead to conflicts among the functions.

I'd recommend trying to pipe things, whenever possible. So, for example,

countscreen25 <- tweets %>% 
  count(screen_name) %>% 
  arrange(desc(n)) %>% 
  slice(1:25)

Everything else looks great! Well done!

kgrove10 commented 5 years ago

Thank you for your advice and feedback! I loaded the utf8 library because it came up as an error ("loadNamespace(name) : there is no package called ‘utf8’") when I was trying to use the Tidyverse (not sure why, but that is what stack exchange recommended to fix the error!).

datalorax commented 5 years ago

Ah… that would be because you didn’t have the package installed, and it’s apparently a dependency of one of the tidyverse packages. So it was good to install it, but you don’t actually need to load it.

On Jan 10, 2019, at 3:45 PM, kgrove10 notifications@github.com wrote:

Thank you for your advice and feedback! I loaded the utf8 library because it came up as an error ("loadNamespace(name) : there is no package called ‘utf8’") when I was trying to use the Tidyverse (not sure why, but that is what stack exchange recommended to fix the error!).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jkosie/lab1/issues/1#issuecomment-453304914, or mute the thread https://github.com/notifications/unsubscribe-auth/AKb-iN1qLyY9vjbknVkXoDYYWmp3aQcCks5vB9CQgaJpZM4Z6eOZ.

kgrove10 commented 5 years ago

Oh, that's good to know. Thank you!