Irisfee / r3final

dashboard:
https://irisfee.github.io/r3final/
1 stars 3 forks source link

Peer Review #3

Open shainatrevino opened 5 years ago

shainatrevino commented 5 years ago

Hi @Irisfee and @xiy1992

First off, amazing job on this project! It is clear that you both are very knowledgeable about manipulating various forms of data in R. I was very impressed! That being said, a lot of your code was above my skill level in R, so it was a little difficult for me to critically evaluate parts of your code that was very complex. I did not make any changes to your actual code, but just added in a few minor comments and noted an error message I got when trying to run the plot codes.

The strengths that I noticed:

Things I learned from your script:

Areas for improvement:

Let me know if you have any questions about this.

Best, Shaina

@datalorax

Irisfee commented 5 years ago

Hi @shainatrevino ,

Thanks for reviewing our codes and thanks for all your suggestions!

The problem you mentioned about the afex and emmeans also happened while others running the codes, and it seems that installing the emmeans again and re-library it solve the problem (We don't know why since we didn't get this error).

we agree we definitely need more comments and explanations! sorry for the confusions while reading our not-well-commented codes.

Best, Xi & Yufei

datalorax commented 5 years ago

Thanks Shaina! Really nice review. As a bit of a clarifier, sprintf uses "C-style" string operations. It's a little complex but basically ends up working a lot like glue::glue instead of paste. See here for an okay, but not great, explanation: https://www.r-bloggers.com/paste-paste0-and-sprintf/

I like the suggestion to add in a sidebar. Explaining acronyms is also definitely important.