VincentAlcazer / StatAid

StatAid is a Shiny app built in an R package to guide researchers and clinicians through data analysis
Other
5 stars 2 forks source link

JOSS Review #1

Closed adithirgis closed 4 years ago

adithirgis commented 4 years ago

Hello, Thanks for submitting this to JOSS and inviting me to review. I found the software very useful. Very good work. Congratulations. I am adding a few comments for this below:

  1. I am not able to see the page correctly there is a overlap between the mobile/drag and drop menu in the right and the table area for the Data Loading tab. (I am using a Windows system). Any thoughts about this? Can the table to be placed a little lower? image

  2. I tried to run this app without the any data upload : It shows me this error for all combinations dropdown items selected and running the Descriptive Table analysis in Data Exploration tab. Am I selecting the wrong options? image

  3. Let me know if I am doing anything wrong here: I am trying to run GAM and LOESS for two numeric variables I get this error. image image

  4. Please add Community Guidelines to the README.md file: stating how to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support.

  5. I see that the description and walk-through is well written inside the application interface. It would be very helpful for other users not in the medical/clinical research to use this package for their own research. A descriptive README.md containing example usage and functionality documentation would be very helpful. I strongly suggest to write a descriptive README.md in your repository. Also, this might be a general comment: Is there a possibility to add description of the pre-loaded data base columns to understand the working of the package better.

  6. Are there any other software similar to StatAid existing already? It will be useful to add a comparison here.

  7. You can also list the academic papers in the API Documentation/ README.md where StatAid has been cited. As stated in the paper.md: "It has already been used by multiple researchers and students for their PhD or medical doctors for their thesis."

  8. I think the good practice of writing cleaner consistent code can be implemented here using these wonderful resources (I too found out during my paper review). Implementing 80 or 120 characters in a line and spacing consistency, see http://adv-r.had.co.nz/Style.html and https://devguide.ropensci.org/building.html#code-style.

  9. I would also suggest to do a check using this command- devtools::check(). I came across three warnings(). To solve those you can add some of the build ignore files into this file. There is also a import from namespaces issue/warning which can be probably solved using this.

  10. A minor addition in the installation here would be more clear:

    # install.packages("remotes")
    # remotes::install_github("VincentAlcazer/StatAid")
    # StatAid::run_app()
  11. I would recommend to more software usage in the paper.md file.

  12. I would also suggest to add a few more automated tests other than the recommended one in golem. Please let me know if they are already added and if I skipped it.

  13. I downloaded the data sets and found that the IRIS data set was not giving desired results I was not able to select any numeric column for regression and correlation or run lm/gam/loess. Are there any other checks to be done before we enter new data into the software other than those mentioned in the software Introduction tab. image

Let me know if you have any further questions. Thanks again, Stay Safe and Regards Adithi

VincentAlcazer commented 4 years ago

Dear @adithirgis, thank you very much for your review and your comments. I will take the time to answer point by point and correct the issues you raised.

VincentAlcazer commented 4 years ago

Dear @adithirgis ,

Thank you for your review and your positive feedback. Your comments helped me to improve the software with a new version I just uploaded to GitHub (1.01). Unfortunately, I did not manage to reproduce some errors you get. Here is a point-by-point answer. I hope we will manage to find where these errors came from.

Thank you again for your time.

Best regards,

Vincent

  1. The table area have been moved down as suggested, with the addition of a “frequent issue” section.

  2. This is working for me both in local and on the website (see picture for expected behavior). I am trying to see where this error came from: Have you tried to perform the analysis on a fresh launch? Did you load another dataset before loading again the base dataset? Are your packages up to date? 2

  3. Same comment as for the 2nd point (see picture for expected result). 3

  4. Additional description and guidelines for these points have been added at the end of the Readme.md.

  5. Thank you for this suggestion. This is indeed an important point and I am currently considering writing a complete tutorial for the software. A short description of the dataset has been added.

  6. To my knowledge, no free open-source software for data analysis exist. Of course, a lot of alternative solutions exist but they are else private software or shareware, or partial webtools with few options. I made a quick additional search on the web and did not manage to find direct concurrent in the same format as StatAid (if you have anything in mind please let me know so I can compare them).

  7. Few papers where StatAid was useful have been published yet. The only one would be Clin Lung Cancer. 2020 May 8:S1525-7304(20)30136-4. doi: 10.1016/j.cllc.2020.04.013, where the software has been used for data exploration but not cited (I was added as an author). The PhD and medical thesis for which StatAid was more thoroughly used are else not going to be published, or with an article in preparation and I hope StatAid will be citable before the submission of these latter works.

  8. The styler package has been used to reformat the whole code in a cleaner way.

  9. I already run this tool before submission and ran it again with no warnings (only 3 notes). I wonder if a location issue could cause this and be potentially related to the 2nd and 3rd issue. Could you please paste me the warnings you had?

  10. The recommended correction has been added.

  11. From what I understood, the paper.md has to be short and should not contain API functionality (from JOSS recommendations: “Given this format, a “full length” paper is not permitted, and software documentation such as API (Application Programming Interface) functionality should not be in the paper and instead should be outlined in the software documentation.”). This point could probably be related to 5, and I am currently working on a more complete tutorial/manual for the software.

  12. The rhub::check_for_cran() test has also been run, with 4 notes and no warnings. Do you have a particular test in mind you would like to see here?

  13. The decimal separator for the Iris dataset is a period (instead of a comma for the original dataset). You have to make the change in the data loading menu so numerical variables are not considered as categorical ones. I added a section in “frequent issues” for this as this could be a regular issue.

adithirgis commented 4 years ago

Hello @VincentAlcazer !

Thanks. Sorry for the late response.

Please treat the following as minor comments / optional-

Feel free to let me know if you have questions. All the best! I will surely try to use this for air quality data! Thanks a lot. Regards and Stay safe Adithi

VincentAlcazer commented 4 years ago

Dear @adithirgis,

Thank you for your answer and your patience. I just uploaded a new version (1.02) integrating @nistara comments and some of your remaining reported issues. Three big points are still pending (the GAM/LOESS issue that I tried to correct, the guide/tutorial and the need for more test that are still work in progress) and I hope I will manage to provide a full correction soon.

Thank you again for your time and your help improving StatAid.

Best regards,

Vincent

2-3. For the GAM/LOESS issue, I tried to force the loading of the mgcv package at the app start: could you please tell me if you still have the issue?

  1. The readme file with the tutorial/guide is still work in progress. I will let you know when it will be ready.
  2. A sentence has been added in the paper.md: “To my knowledge, no free open-source software directly designed for researchers with an intuitive interface and a collaborative/evolving environment has been proposed yet.”
  3. I removed the statement of previous use as I have no citations/proof to provide.
  4. The testing part has also been commented by @nistara’s review. As I explained, I hardly see what kind of test I could add to my functions for the moment in the way I designed them (i.e. each function take the dataframe and the columns as input to produce the results, making the dataframe loading the limiting step). I am currently working on testing solutions and will try to propose relevant test with testthat for the next corrections.

Minor comments:

adithirgis commented 4 years ago

Hi @VincentAlcazer!

Thanks for the response! I will take a look again and wrap the review process once the tutorial is updated.

Take care, Adithi

VincentAlcazer commented 4 years ago

Dear @adithirgis ,

I hope your are doing well.

I wanted to inform you that I managed to correct the three remaining points of the review. You will find in the updated v1.03:

Thanks again for your time and comments. I'm looking forward to your feedback.

Best regards,

Vincent

adithirgis commented 4 years ago

Hi @VincentAlcazer!

Sorry for getting back late.

Thanks again for all the changes, the app has been improved a lot. I see that all the comments have been implemented. I will wrap the review process soon.

Could you please share what changes did you make to fix the GAM/LOESS analysis (it works on both Cloud and local system now!)? Thanks for such a wonderful user guide! Tests also seem to be added for functions! Please continue adding the tests to the test folder to improve the application time and again.

Regards, Take care! Adithi

adithirgis commented 4 years ago

Hi @VincentAlcazer!

Congratulations for the amazing application. I will wrap the review process now.

All the best and take care! Adithi

VincentAlcazer commented 4 years ago

Hi @VincentAlcazer!

Sorry for getting back late.

Thanks again for all the changes, the app has been improved a lot. I see that all the comments have been implemented. I will wrap the review process soon.

Could you please share what changes did you make to fix the GAM/LOESS analysis (it works on both Cloud and local system now!)? Thanks for such a wonderful user guide! Tests also seem to be added for functions! Please continue adding the tests to the test folder to improve the application time and again.

Regards, Take care! Adithi

The broom::augment function was not supporting the fit object from GAM so I had to manually recode the augment dataframe output by my function. However, I still do not understand why it worked on shinyapps.io and on my local computer...! Thanks for providing me the Rcloud resource that helped me fix this!