search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
710 stars 38 forks source link

[REVIEW]: ‘Bioassays’; a new package in R for analyzing cellular assays #2402

Closed whedon closed 3 years ago

whedon commented 4 years ago

Submitting author: @anwarbio (ANWAR PALAKKAN) Repository: https://github.com/anwarbio/bioassays Version: v1.0.1 Editor: @fboehm Reviewers: @tomsing1, @homonecloco Archive: 10.5281/zenodo.4075110

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/68721e17fcb3d07413631c8d395dff5c"><img src="https://joss.theoj.org/papers/68721e17fcb3d07413631c8d395dff5c/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/68721e17fcb3d07413631c8d395dff5c/status.svg)](https://joss.theoj.org/papers/68721e17fcb3d07413631c8d395dff5c)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@tomsing1, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @fboehm know.

✨ Please try and complete your review in the next six weeks ✨

Review checklist for @tomsing1

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

The vignettes needed proofreading. I have suggested changes and submitted a pull request for the author to review. (I also included a few other minor suggestions in the pull request, which he might find useful.)

Review checklist for @homonecloco

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @tomsing1 it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 4 years ago

PDF failed to compile for issue #2402 with the following error:

/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:377:in parse': (tmp/2402/paper.md): mapping values are not allowed in this context at line 2 column 17 (Psych::SyntaxError) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:377:inparse_stream' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:325:in parse' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:252:inload' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:473:in block in load_file' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:472:inopen' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:472:in load_file' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-446a0298a33b/lib/whedon.rb:125:inload_yaml' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-446a0298a33b/lib/whedon.rb:85:in initialize' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-446a0298a33b/lib/whedon/processor.rb:36:innew' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-446a0298a33b/lib/whedon/processor.rb:36:in set_paper' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-446a0298a33b/bin/whedon:55:inprepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:in run' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:ininvoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:in dispatch' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:instart' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-446a0298a33b/bin/whedon:119:in <top (required)>' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:inload' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in `

'

arfon commented 4 years ago

@anwarbio - your paper title needs quoting, i.e. change this: 

title: Bioassays: a new package in R for analyzing cellular assays

to this: 

title: "Bioassays: a new package in R for analyzing cellular assays"

in your paper.md.

anwarbio commented 4 years ago

@arfon - The above change is made in the title of paper.md

arfon commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

anwarbio commented 4 years ago

@whedon, @arfon Few mistakes were in the article proof, and is now corrected in paper.md. Details of the correction are 1)Line 25 1st paragraph last line: high-throughput is corrected as High-throughput 2) Line 27 2nd paragraph 3rd line: us is corrected as use 3) Line 29 3rd paragraph 1st line: server is corrected as serve 4) Line 39 7th paragraph 1st line: An extra convert is deleted. Thanking you, Anwar

whedon commented 4 years ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands
fboehm commented 4 years ago

Thanks, @anwarbio.

fboehm commented 4 years ago

@whedon remove homoneclocom as reviewer

whedon commented 4 years ago

OK, homoneclocom is no longer a reviewer

fboehm commented 4 years ago

@whedon add @homonecloco as reviewer

whedon commented 4 years ago

OK, @homonecloco is now a reviewer

tomsing1 commented 4 years ago

@fboehm I think I waited too long to accept the https://github.com/openjournals/joss-reviews/invitations invite. Would you mind refreshing it so I can edit the checklist? Many thanks!

danielskatz commented 4 years ago

@whedon re-invite @tomsing1 as reviewer

whedon commented 4 years ago

OK, the reviewer has been re-invited.

@tomsing1 please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

homonecloco commented 4 years ago

Hi @anwarbio , I started the review with the documentation. I am still to read the paper, but I'll give you my comments of what I have reviewed so far, so you can consider them.

  1. The API documentation (https://cran.r-project.org/web/packages/bioassays/bioassays.pdf) has the beginning looking incomplete or misformated. The documentation of the actual functions seems ok, but having so many NA at the beginning of the document is not recommended.
  2. The vignette with examples (https://cran.r-project.org/web/packages/bioassays/vignettes/bioassays-examples.html) refers to some files, like “L DIFF P3 72HRS.csv”. I couldn’t find that file in the repository. I appreciate that the examples may be included as part of the package, but for this particular document, having the actual file used can help the users to format their tables.
  3. In some packages, the documentation is in html and in PDF. It would be useful to have the same option for the vignettes of this packages. The PDFs are hard to copy and paste.
anwarbio commented 4 years ago

Hi @homonecloco

Thank you for reviewing the package and your suggestions are really helpful in improving the package. I have made the below changes in github files. Once all are happy with contents, then I can go for an update on CRAN with all changes.

  1. The API documentation (https://cran.r-project.org/web/packages/bioassays/bioassays.pdf) has the beginning looking incomplete or misformated. The documentation of the actual functions seems ok, but having so many NA at the beginning of the document is not recommended. comment: I have now corrected this

  2. The vignette with examples (https://cran.r-project.org/web/packages/bioassays/vignettes/bioassays-examples.html) refers to some files, like “L DIFF P3 72HRS.csv”. I couldn’t find that file in the repository. I appreciate that the examples may be included as part of the package, but for this particular document, having the actual file used can help the users to format their tables. comment: All the files used in the examples are now added in the exdata folder and is now mentioned in 'vignette with examples'

  3. In some packages, the documentation is in html and in PDF. It would be useful to have the same option for the vignettes of this packages. The PDFs are hard to copy and paste. comment: The vignettes are in html format. Adding both html and pdf file might be difficult as the images of 'heatplate' appear in different dimensions in pdf and html. I might need separate rmd files for html and pdf, I doubt if CRAN will allow this.

Thanking you, Amwar

homonecloco commented 4 years ago

Hi @anwarbio , Thanks for making the updates so fast. Is there a way I can preview them? Specially I would like to see the NAs go away. In any case, I see you uploaded the file, and it is fine if you only have the html documentation. As long it is available in either format, in an easy to copy and paste (so you can test that the commands are working as expected), is fine. Best, Ricardo.

anwarbio commented 4 years ago

Hi @homonecloco , Thank you for the reply. I have made few more edits today to put each function in separate line (you can see them in the github). I do not know how to share the preview file, so I have shared you a screenshot showing the functions arranged neatly. Thanking you, Anwar

Picture1

fboehm commented 4 years ago

@tomsing1 and @homonecloco - I hope that your reviews are going well. Please feel free to complete the checklists when you're satisfied with the submission and to suggest revisions until you're satisfied. Please let me know how I might help.

fboehm commented 4 years ago

@tomsing1 and @homonecloco - I see that @tomsing1 opened three issues in the package repository: https://github.com/anwarbio/bioassays

@anwarbio responded to the three issues - should we consider the issues closed? Or is more discussion needed?

Thanks again!

homonecloco commented 4 years ago

In general, I like the extensions to ggplot to display the plates. I can see this plots useful by themselves. It is nice to have some functions that force the structure of the data to the input of the plots, but I don’t think it is completely necessary for a user proficient in R.

anwarbio commented 4 years ago

Hi @homonecloco,

Thank you for the comments. I have made correction as suggested. Details are given below.

  1. In general, I like the extensions to ggplot to display the plates. I can see this plots useful by themselves. It is nice to have some functions that force the structure of the data to the input of the plots, but I don’t think it is completely necessary for a user proficient in R. Reply: Any multi-well plate data can be structured for the plots (function 'heatplate') by using functions 'data2plateformat', 'plate2df', 'matrix96' alone or in combinations (depending on the input data). I have included this as a new sentence in line 41

  2. There is a contradiction when comparing the software with DRfit. I would remove the comparison, as DRfit is designed to be user friendly. It may not scale well, but it is not designed for programmers. Also, in the last part of the summary, Bioassays claims to be customisable by anyone using R, which is equivalent to knowing Java to extend Drift. Reply: In line 29 the sentence "They are also limited to the options provided by the software, unless a user is willing to write additional JAVA code, a skill not particularly common among cell biologists" is now removed.

  3. The core text oversells the usability of the packages. It is indeed consistent with the user experience that you would get in R, but it is not as “user-friendly” as a graphical interfase. I would tone down the language. For example, the phrase "Since even the most naive computer users know how to change the names of files, this provides a very simple way to pass data of this type into the analysis.”, reads condescending. Reply: This sentence (line 37) is now modified as "This function provides a very easy way to pass data of this type into the analysis by simply editing file names." Another sentence in line 31 (Moreover, it is strongly documented in a manner designed to be easy for even beginners to grasp.) is modified as "Moreover, we have provided examples (bioassays-examples) in the vignettes, which will be very helpful for beginners to grasp how this package can be used."

  4. In Acknowledgment, the frase "Support from lab members are acknolegment.” Has a spelling error, and it is not clear which lab members are acknloedege, or at least to which research group. Reply: The spelling mistake in acknowledgment is now corrected. Lab details are also provided (line 50).

  5. The contributions.md stats that there is continues integration in Travis, can you make the link more accessible, to be able to monitor if the current state of the code is running? Reply: The link is now updated.

Thanking you, Anwar

fboehm commented 4 years ago

@anwarbio thank you for the updates! @homonecloco and @tomsing1 - Thanks for your work with the reviews. If you find that you can't yet check some of the boxes in the checklist, please continue to suggest improvements. Please let me know if I can help with anything. Thanks again!!

homonecloco commented 4 years ago

@fboehm , can you produce the PDF again, so I can check the changes?

fboehm commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

PDF failed to compile for issue #2402 with the following error:

pandoc-citeproc: reference homonecloco not found Error producing PDF. ! TeX capacity exceeded, sorry [input stack size=5000]. \reserved@a ->\def \reserved@a *{\let \@xs@assign \@xs@expand@and@detokenize... l.331 }

Looks like we failed to compile the PDF

fboehm commented 4 years ago

@anwarbio - I failed to produce the pdf. It looks like there's a problem with pandoc-citeproc. By chance did you use the text "@homonecloco" somewhere in your document? If so, you might need to specify that you aren't intending to cite the key "homonecloco" in your text. You might be able to do this by preceding the at sign by two backslashes, but I could be mistaken on that.

arfon commented 4 years ago

I can fix the issue here folks, looks like Pandoc is getting confused with the reviewer handles and citations. Pls hold...

arfon commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

fboehm commented 4 years ago

Thanks, @arfon !

fboehm commented 4 years ago

@homonecloco and @tomsing1 - we have the updated pdf now. What are your feelings about it and the package in the current state? As you work through the last of the checklists, please feel free to continue offering suggestions for improvement as needed, and please do let me know how I can assist you. Thanks again!!

fboehm commented 4 years ago

@homonecloco and @tomsing1 - I hope that your reviews are progressing well. Is there something that I can assist with right now? With help from Arfon, we've created the pdf. Do you feel that additional revisions are needed? Are there any checkboxes that need attention from @anwarbio ? Thanks again!!

fboehm commented 4 years ago

@tomsing1 and @homonecloco - It seems that there hasn't been much activity on this review over the last couple of weeks. When you have a moment, would you see if you're comfortable with checking more of the boxes from the checklist? If you're not comfortable checking some boxes, we should be sure that @anwarbio is aware of your concerns. Thanks again!

homonecloco commented 4 years ago

Hi, I'm sorry for not commenting earlie. I'm mostly satisfied now. The only thing missing from the guideline is a community guideline on how to contribute to the project. A simple statement in the main readme should be enough.

Best, Ricardo.

anwarbio commented 4 years ago

Hi, @homonecloco Thank you very much. I have now added a paragraph on contribute in readme.

Kind regards, Anwar

homonecloco commented 4 years ago

@anwarbio , thanks for the prompt update. @fboehm , I can canfirm that, from my side, this project meets all the requirements in the guideline.

anwarbio commented 4 years ago

Hi @tomsing1 , Please let me know the concerns you may have. I am happy to consider them and make necessary modifications. Thanks, Anwar

fboehm commented 4 years ago

Thanks, @homonecloco, for your review! @tomsing1 - how is your review progressing? Please let me know if I can assist with something. Thanks again!

tomsing1 commented 3 years ago

Dear @fboehm and @anwarbio, I have now finished my review. (Thank you for your patience.) I suggested a few improvements in this pull request. I mainly corrected spelling or grammar mistakes in the vignettes and also added the .Rbuildignore file. (I also found one last typo in the paper itself.) The manuscript now fulfills all of the requirements on the list. Good to go!

anwarbio commented 3 years ago

Dear @tomsing1 , Thank you very much for the corrections. I have now applied these changes. Thanks!

fboehm commented 3 years ago

Thank you, @tomsing1 and @anwarbio !

fboehm commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

fboehm commented 3 years ago

@whedon check references

whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1007/s10295-006-0086-3 is OK
- 10.1002/em.22014 is OK
- 10.1007/978-1-4939-2742-5_11 is OK
- 10.1016/j.bios.2015.05.018 is OK
- 10.1038/srep42383 is OK
- 10.1093/bioinformatics/btr664 is OK
- 10.1186/s12859-019-2891-5 is OK

MISSING DOIs

- None

INVALID DOIs

- NBK144065 is INVALID
fboehm commented 3 years ago

@anwarbio - whedon suggests that NBK144065 is an invalid doi. Can you double-check this doi?

anwarbio commented 3 years ago

@fboehm - This is a eBook reference and do not have a doi. Reference is now corrected