search

openjournals / joss-reviews

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

[REVIEW]: stanscofi and benchscofi: a new standard for drug repurposing by collaborative filtering #5973

Closed editorialbot closed 5 months ago

editorialbot commented 8 months ago

Submitting author: !--author-handle-->@clreda<!--end-author-handle-- (Clémence Réda) Repository: https://github.com/RECeSS-EU-Project/stanscofi/ Branch with paper.md (empty if default branch): Version: v.2.0.0, v.2.0.0 Editor: !--editor-->@Nikoleta-v3<!--end-editor-- Reviewers: @jaybee84, @abhishektiwari Archive: 10.5281/zenodo.10561760

Status

status

Status badge code:

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

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

@jaybee84 & @abhishektiwari, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

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

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

📝 Checklist for @abhishektiwari

📝 Checklist for @jaybee84

editorialbot commented 8 months ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

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

@editorialbot commands

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

@editorialbot generate pdf
editorialbot commented 8 months ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.05 s (722.9 files/s, 142478.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          13            281           1120           1526
TeX                              1             38              0            343
Jupyter Notebook                 1              0           3115            338
Markdown                         3             87              0            235
YAML                             5             19             15            193
JSON                             3              0              0            158
reStructuredText                10             45             41             75
DOS Batch                        1              8              1             26
make                             1              4              7              9
TOML                             1              0              0              3
-------------------------------------------------------------------------------
SUM:                            39            482           4299           2906
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 8 months ago

Wordcount for paper.md is 1752

editorialbot commented 8 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1089/genedge.5.1.39 is OK
- 10.1016/j.apsb.2022.02.002 is OK
- 10.1038/s41598-019-54849-w is OK
- 10.1186/s13321-020-00450-7 is OK
- 10.1016/j.eswa.2017.05.004 is OK
- 10.1093/bib/bbab581 is OK
- 10.1186/s12859-019-2983-2 is OK
- 10.1109/icdm.2010.127 is OK
- 10.1186/s12859-020-03898-4 is OK
- 10.1109/TCBB.2022.3212051 is OK
- 10.1109/TCBB.2023.3254163 is OK
- 10.1016/j.patter.2023.100804 is OK
- 10.1007/3-540-44886-1_25 is OK
- 10.1093/bioinformatics/bty013 is OK
- 10.1093/cid/ciab350 is OK
- 10.5281/zenodo.8038847 is OK
- 10.5281/zenodo.8241505 is OK
- 10.5281/zenodo.7982964 is OK
- 10.5281/zenodo.7982969 is OK
- 10.1038/msb.2011.26 is OK
- 10.1093/bioinformatics/bty013 is OK
- 10.1093/bioinformatics/btw770 is OK
- 10.3389/fphar.2021.784171 is OK
- 10.1093/bioinformatics/btz965 is OK
- 10.1016/j.asoc.2021.107135 is OK
- 10.1093/bioinformatics/btz331 is OK
- 10.1093/bioinformatics/btw228 is OK
- 10.1145/3308558.3313562 is OK

MISSING DOIs

- None

INVALID DOIs

- None
Nikoleta-v3 commented 8 months ago

Hey @jaybee84, @abhishektiwari this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements ✅ As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/5973 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@Nikoleta-v3) if you have any questions/concerns. 😄 🙋🏻

editorialbot commented 8 months ago

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

abhishektiwari commented 8 months ago

Review checklist for @abhishektiwari

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

jaybee84 commented 8 months ago

Review checklist for @jaybee84

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Nikoleta-v3 commented 8 months ago

Hey @abhishektiwari and @jaybee84 👋🏻, any updates on your reviews? 😄

abhishektiwari commented 8 months ago

I will share initial feedback early next week.

Nikoleta-v3 commented 8 months ago

Thank you 🙏🏻 and thank you again for your time!

jaybee84 commented 8 months ago

@Nikoleta-v3 Thanks for the reminder. I will also complete it within the next week.

abhishektiwari commented 8 months ago

@clreda Initial comment on structure of Paper itself.

— Just for consistency purposes, the Summary should come before Statement of need. — When I read current Summary it does not describe the high-level functionality and purpose of the software for a diverse, non-specialist audience. See example paper here.

abhishektiwari commented 8 months ago

@clreda I spent a few hours trying to install the package but neither pip nor conda instructions work (details added on the GitHub issue here). I use Mac M2 laptop and tried running these with Rosetta mode but no luck. Is there a Dockerfile one can use to test these packages?

Can you share what OS is supported by stanscofi?

Update: Added GitHub issue with instructions to run stanscofi as a Docker container. Please feel free to include them for users and/or contributors.

clreda commented 8 months ago

Dear @abhishektiwari, thanks a lot for accepting to review this manuscript!

I have tried to address the issue in the installation, please let me know how it goes. Thank you very much for the instructions to run on Docker, I will add them to the repository and documentation! EDIT: in commit ff312993

As for the comments on the manuscript itself, (I will address those concerns this week) edit: please refer to the newest version of the paper.

clreda commented 8 months ago

@editorialbot generate pdf

editorialbot commented 8 months ago

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

abhishektiwari commented 8 months ago

Thanks, @clreda I have now worked my way through the stanscofi and was able to validate package functionality.

I am now having issue with benchscofi due it's restrictive tensorflow requirements.

Created ticket here: https://github.com/RECeSS-EU-Project/benchscofi/issues/1

clreda commented 7 months ago

I have replied to your issue. I believe the best course of action is to provide for users the alternative installation guide, which relies on pip dependency conflict solver, and keep the current one to ensure smooth automated tests. Please feel free to mention other possibilities which might improve user functionality

Nikoleta-v3 commented 7 months ago

Hello @abhishektiwari @jaybee84👋🏻, any updates regarding your reviews?

@clreda, I can see that https://github.com/RECeSS-EU-Project/stanscofi/issues/2 is now closed. In the future, could you indicate the commit hash (or even better, the pull request) that resolved the issue? It will make reviewers' life a bit easier 😄

abhishektiwari commented 7 months ago

@Nikoleta-v3 I will revisit paper again, and expect final feedback by end of week.

abhishektiwari commented 7 months ago

@clreda Looked at revised paper structure, still not sold on current summary. I generally think software paper summary as an elevator pitch - problem statement, solution, key value proposition, any additional detail for target audience. Revised version mentions the problem statement but not the solution.

clreda commented 7 months ago

@editorialbot generate pdf

editorialbot commented 7 months ago

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

clreda commented 7 months ago

@abhishektiwari Thanks for your feedback! Please find enclosed the newest revision of the paper. Let me know if that tackles your concerns. (commit 196bd5f)

@jaybee84 Thanks for accepting to review this submission! Package cnvkit is not part of the requirements of packages stanscofi nor benchscofi. The only constraints version-wise on these packages is that joblib>=1.0.1, which allows the use of later versions of joblib.

abhishektiwari commented 7 months ago

@clreda thanks for revising the summary.

Additional feedback on installation:

At this point my only non-blocking concern is that installation of benchscofi package is not easy. Due to a large number of dependencies it took a fair bit of my time to install benchscofi. This may discourage people trying to use these packages.

I strongly suggest providing a Docker images for both stanscofi and benchscofi.

Problem become more obvious when you see PyPI release pipeline for benchscofi never succeeded. Author should fix PyPI and add Conda releases for benchscofi. Release pipeline for stanscofi seems better but similar issues. Last build succeeded for PyPI and failed for Conda.

Finally Overall, stanscofi and benchscofi are very useful packages to anyone working on drug screening and repurposing. stanscofi offers end to end workflow for screening whereas benchscofi implements a good number of published algorithms.

@Nikoleta-v3 Completed my review.

jaybee84 commented 7 months ago

@clreda I am trying to review the functionality of benchscofi and having some trouble. I strongly recommend adding documentation about benchscofi that is similar to the jupyter notebook for stanscofi. Once that is added, I should be able to complete my review.

clreda commented 7 months ago

Dear @abhishektiwari, I agree with your point on the installation of benchscofi. There are several cross-platform algorithms, hence the large number of dependencies. I will work on Docker images for both packages, and ensure that the pipelines for PyPI and Conda work for stanscofi and benchscofi in the next weeks. The main issue with including a conda release for benchscofi is that some of the dependencies are not present in conda and would require a script file (ex. build.sh) to download them from PyPI and install them locally during the installation (which I will try to work on in the next weeks).

Dear @jaybee84 Thanks for your feedback! I assume that you managed to test the functionality of stanscofi. May I close the following issue (issue #3)? I don't understand whether the issue with benchscofi is about the installation (and having a similar list of commands to execute benchscofi within Docker and run the notebooks) or having access to an introductory notebook in the first place (in that case, there are several notebooks in the docs/ folder: in particular, one for using algorithms within benchscofi (here) and another one on class prior estimations (here)). Please let me know what is the problem.

jaybee84 commented 7 months ago

Ah! @clreda I see that the issue is that I was not looking at the right place. Since both packages are being released in the same publication, I expected that there would be a pointer from the stanscofi repo to the benchscofi repo so that all documentation for both packages are easy to find from one place. I see now that the RECeSS EU project is the common origin point to navigate to both packages. I will take a look at the benchscofi functionality soon.

jaybee84 commented 6 months ago

@Nikoleta-v3 @clreda I have a minor suggestion for updated documentation for benchscofi in the linked open PR. Once that is merged in, my review should be considered complete with approval for publication.

clreda commented 6 months ago

Dear @jaybee84 thanks for your contribution. I have merged the PR. (commit 0f960a26)

Nikoleta-v3 commented 6 months ago

Thank you very much @jaybee84 & @abhishektiwari, for your thorough reviews and most importantly, for your time!

@clreda, I want to also have one final look over the submission before we proceed. Unfortunately, I am going on a short break this week, but I will try to get back to you as soon as I can 😄

Once again, thank you all for your time and efforts 🙏🏻

clreda commented 6 months ago

Dear @abhishektiwari Docker images for stanscofi and benchscofi are now available on DockerHub. The documentation in both repositories has been updated accordingly. The release pipelines have been fixed (stanscofi: commit 981bef4; benchscofi: commit b41d2d9). Only thing that is missing is the Conda release pipeline for benchscofi.

Nikoleta-v3 commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

Nikoleta-v3 commented 6 months ago

Thank you for your patience @clreda, and thanks to the reviewers for their time! I believe both their comments greatly benefited the paper, especially regarding installation. I have a few comments/suggestions.

Documentation:

Regarding documentation, I still think something is missing. I realize that the packages have two separate repositories for a purpose, but have you ever considered having a common Read the Docs?

My issue is that https://recess-eu-project.github.io/stanscofi/ has very minimal information. Everything in Introduction to stanscofi.ipynb should be part of the online documentation. I shouldn't have to clone the repo to see the examples. You can open the Jupyter notebook on GitHub, but then I can't navigate to the sections I want by clicking. So, it just seems a bit too much for the user end. The benchscofi package doesn't have online documentation but has a few Jupyter notebooks, which is why I am suggesting a common online document. What do you think?

FYI, it doesn't have to be on Read the Docs. I see that you also have this: https://recess-eu-project.github.io/. It could be anywhere. At the moment, the online documentation just doesn't add much.

Other comments:

clreda commented 5 months ago

Dear @Nikoleta-v3, thanks for your feedback. I hope your concerns regarding documentation are now addressed (commit df887f7 in stanscofi; commit 8bedfa1 in benchscofi). In particular, both documentations are now present here, with tutorials and examples.

I have also updated the README in benchscofi to highlight the automated tests performed for algorithmic contributions. I have also fixed the type in the paper.

Please let me know if you have other questions.

clreda commented 5 months ago

@editorialbot generate pdf

editorialbot commented 5 months ago

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

Nikoleta-v3 commented 5 months ago

The documentation looks great! Thank you for all your work @clreda.

At this point could you please:

I can then move forward with accepting the submission.

clreda commented 5 months ago

Dear @Nikoleta-v3, thanks for your praising.

Release: stanscofi v2.0.0 Archive: "A new standard for drug repurposing by collaborative filtering: stanscofi and benchscofi" v2.0.0 DOI: 10.5281/zenodo.10549119

Please let me know if anything else is needed.

Nikoleta-v3 commented 5 months ago

@editorialbot set v.2.0.0 as version

editorialbot commented 5 months ago

Done! version is now v.2.0.0

Nikoleta-v3 commented 5 months ago

@editorialbot check references

editorialbot commented 5 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1089/genedge.5.1.39 is OK
- 10.1016/j.apsb.2022.02.002 is OK
- 10.1038/s41598-019-54849-w is OK
- 10.1186/s13321-020-00450-7 is OK
- 10.1016/j.eswa.2017.05.004 is OK
- 10.1093/bib/bbab581 is OK
- 10.1186/s12859-019-2983-2 is OK
- 10.1109/icdm.2010.127 is OK
- 10.1186/s12859-020-03898-4 is OK
- 10.1109/TCBB.2022.3212051 is OK
- 10.1109/TCBB.2023.3254163 is OK
- 10.1016/j.patter.2023.100804 is OK
- 10.1007/3-540-44886-1_25 is OK
- 10.1093/bioinformatics/bty013 is OK
- 10.1093/cid/ciab350 is OK
- 10.5281/zenodo.8038847 is OK
- 10.5281/zenodo.8241505 is OK
- 10.5281/zenodo.7982964 is OK
- 10.5281/zenodo.7982969 is OK
- 10.1038/msb.2011.26 is OK
- 10.1093/bioinformatics/bty013 is OK
- 10.1093/bioinformatics/btw770 is OK
- 10.3389/fphar.2021.784171 is OK
- 10.1093/bioinformatics/btz965 is OK
- 10.1016/j.asoc.2021.107135 is OK
- 10.1093/bioinformatics/btz331 is OK
- 10.1093/bioinformatics/btw228 is OK
- 10.1145/3308558.3313562 is OK

MISSING DOIs

- None

INVALID DOIs

- None
Nikoleta-v3 commented 5 months ago

@editorialbot set 10.5281/zenodo.10549119 as archive

editorialbot commented 5 months ago

Done! archive is now 10.5281/zenodo.10549119

Nikoleta-v3 commented 5 months ago

@editorialbot generate pdf

editorialbot commented 5 months ago

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

Nikoleta-v3 commented 5 months ago

@editorialbot recommend-accept