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

JOSS Review 1 #52

Closed
pierrerogy opened this issue Feb 24, 2022 · 3 comments
Closed

JOSS Review 1 #52

pierrerogy opened this issue Feb 24, 2022 · 3 comments

Comments

@pierrerogy
Copy link

Hello!

First, great job on the apps, they look great and I am sure they will indeed be a fundamental step in teaching data science. I found the documentation thorough and clear, and the apps engaging and fun to use. Please find below my comments associated with the checklist in the JOSS review. Please let me know if you have any questions!

General checks

Everything looks good!

Functionality & Documentation

I was able to download the repo successfully using the provided command. However, I had several issues with the rest of the provided code.

  • The package thatssorandom is not available on at least the two most recent versions of R, so I had to use install.github(“https://github.com/EdwinTh/thatssorandom") to get it and use the apps
  • I could not run the app using the provided command (shiny::runApp("/path/to/wi-fast-stats")). I think the first issue here is that the repo downloads as fast-stats and not wi-fast-stats. Then, the provided command should directly point to the folder where one of the app is present in shiny-app/.
  • I think the code could use a bit more commenting. It took me some time to wrap my head around what was happening. More importantly, as this app is intended to a younger audience, it could be their entry door to coding. If the code is adequately commented, it will definitely be more understandable by students (and teachers/researchers!) and thus have broader impact.

In terms of the app functionnality, I only have two main comments (only using sample data):

  • I think use of the app would be made a lot easier if the "Group variable" and "Quantity" radiobuttons could only show the corresponding variables, instead of all the columns (categorical and numerical variables, respectively).
  • It may be worth it to let plant ID be a "Group variable"? I am not sure if yes, but my data mind would love to see a raw data plot or a histogram.

Software paper

  • Williams and Hill (1986) is missing DOI: 10.1126/science.232.4756.1385
  • The paper refers to a section called "Data summary" but the apps have either nothing or "Data upload", I would make section names constistent.


Best of luck on the rest, and can't wait to see this project completed!

Pierre

@yizhou0522
Copy link
Collaborator

Hi @pierrerogy @xuanxu ,

I'm Yizhou, one of the authors of this app. Thank you so much for the constructive comments on our work. I'll use them to improve our work and submit a more completed version later. By the way, are there any deadlines for us to resubmit our revised work?

@crsl4
Copy link
Owner

crsl4 commented Feb 26, 2022

Echoing Yizhou, thanks so much for the constructive feedback!
@JasonJWilliamsNY should we wait to receive both reviews to make changes or should we address them as they come?
Thanks!

@magitz magitz mentioned this issue Mar 7, 2022
14 tasks
@crsl4
Copy link
Owner

crsl4 commented Mar 20, 2022

We thank the reviewers for such helpful comments that have greatly improved the web apps and the manuscript. All of our changes are in the review branch which we will merge to master after approval by the reviewers and editorial team.

Review 1

Functionality and Documentation:

  • We double checked all the steps in the readme file to run the web apps locally, and provided more details on how to install thatssorandom and how to run the shiny::runApp command
  • We have added more comments to the code and we have added a new section on the documentation for young audiences interested in coding
  • We modified the interface so that only categorical variables appear under "Group variable" and only quantitative variables appear under "quantity"
  • For the webinar-aug20 sample data, we make plant ID a group variable. We found that for the other web apps, it did not make sense to change plant ID as categorical variable because some plots became not very intepretable, so we decided to keep them as quantitiative variables.

Software paper

  • We added the DOI to me Williams and Hill (1986) paper
  • We now make reference to the "Data Upload" section in the paper instead of the "Data Summary"

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

No branches or pull requests

3 participants