search

openjournals / joss-reviews

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

[REVIEW]: HARDy: Handling Arbitrary Recognition of Data in Python #3829

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: !--author-handle-->@MariaPoliti<!--end-author-handle-- (Maria Politi) Repository: https://github.com/EISy-as-Py/hardy Branch with paper.md (empty if default branch): Version: v1.0.1 Editor: !--editor-->@gkthiruvathukal<!--end-editor-- Reviewers: @andrewtarzia, @alberto-battistel Archive: 10.5281/zenodo.6339767

: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/6bcfb38bd849bb22b02615607236a98d"><img src="https://joss.theoj.org/papers/6bcfb38bd849bb22b02615607236a98d/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/6bcfb38bd849bb22b02615607236a98d/status.svg)](https://joss.theoj.org/papers/6bcfb38bd849bb22b02615607236a98d)

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

@andrewtarzia & @alberto-battistel, 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 @gkthiruvathukal 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 ✨

Review checklist for @andrewtarzia

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @alberto-battistel

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @andrewtarzia, @alberto-battistel 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 3 years ago

Wordcount for paper.md is 1378

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

OK DOIs

- 10.1021/acs.chemmater.9b03043.s001 is OK
- 10.1002/aenm.201900555 is OK
- 10.1109/dsaa.2015.7344858 is OK
- 10.24963/ijcai.2017/352 is OK
- 10.1107/s1600576720000552 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.11 s (611.6 files/s, 120428.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          30            845           1884           2675
Markdown                        10            186              0            560
YAML                            11            112            136            481
Jupyter Notebook                 4              0           5756            133
reStructuredText                 9             81             90            118
TeX                              1              7              0             73
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            67           1243           7874           4075
-------------------------------------------------------------------------------

Statistical information for the repository '653737375ea3b5c6c4ffd45d' was
gathered on 2021/10/16.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Abdul Moeez                      1            51             62            0.14
David Hurt                      55          3545           1093            5.76
Maria Politi                    89         19618           4301           29.71
MariaPoliti                      7           222            425            0.80
Moeez                            2             2              2            0.00
amoeezuw                       136         26172          25012           63.58

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Abdul Moeez                  19           37.3         15.6               21.05
David Hurt                  969           27.3         15.2               15.79
Maria Politi               2725           13.9         14.2                7.08
amoeezuw                   1696            6.5         12.4               13.03
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:

andrewtarzia commented 3 years ago

I have read the manuscript and very excited to review and test out this software!

Created some issues about getting started and installation, which has not been smooth for me - although I acknowledge that it could be because I have not used TensorFlow before and I am not used to its installation: https://github.com/EISy-as-Py/hardy/issues/9 https://github.com/EISy-as-Py/hardy/issues/10 https://github.com/EISy-as-Py/hardy/issues/11

Comments on the manuscript:

whedon commented 3 years ago

:wave: @alberto-battistel, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @andrewtarzia, please update us on how your review is going (this is an automated reminder).

andrewtarzia commented 3 years ago

I have read the manuscript and very excited to review and test out this software!

Created some issues about getting started and installation, which has not been smooth for me - although I acknowledge that it could be because I have not used TensorFlow before and I am not used to its installation: EISy-as-Py/hardy#9 EISy-as-Py/hardy#10 EISy-as-Py/hardy#11

Comments on the manuscript:

  • The figures are a little unclear. Some explanation of Fig 1 would be great.
  • Explanation of the left-most column in Fig 2 and the distinction between symbols in Figure 3 are necessary.

The above has mostly been answered. Still awaiting a way to test parts of Hardy that do not require the costly training that I cannot perform (issue 11). At this point, it is still unclear how to use Hardy after training a model that can make the classifications.

The checklist above highlights what is missing.

andrewtarzia commented 2 years ago

I have completed my review and recommend acceptance of this software paper, the work is potentially very useful, although some checklist items above remain unticked for me, which I highlight here:

Google colab notebooks have been developed throughout this review process to answer issues I have sent that show the analysis and use of models from hardy. These guides should be formalised and made easy to find from the README and docs.

This goes with the issue above, I think the documentation is scattered in different forms (examples, documentation and READMEs) and is difficult to grasp. Additionally, many functions/methods lack complete documentation (e.g., data_wrapper in run_hardy.py)

The writing is good and the manuscript quality is overall good, just some minor comments I highlighted earlier:

  • The figures are a little unclear. Some explanation of Fig 1 would be great.
  • Explanation of the left most column in Fig 2 and the distinction between symbols in Figure 3 are necessary.
MariaPoliti commented 2 years ago

@andrewtarzia Thank you so much for the feedback and the review you provided for this paper. The remaining points on your checklist have been addressed:

@gkthiruvathukal adn @alberto-battistel : Is there any update for this publication? When can we expect the review to be completed?

Thank you again all for your work and your feedback on this software!

gkthiruvathukal commented 2 years ago

@MariaPoliti I am just returning from my "out of office", so I should be able to follow up shortly (hopefully within 24-48 hours). Thanks for the gentle nudge!

gkthiruvathukal commented 2 years ago

@andrewtarzia Thanks for your feedback. @MariaPoliti Thanks for addressing @andrewtarzia's feedback.

@alberto-battistel Have you been able to review this submission? I don't see any input on the checklist or comments. We need input from you to complete this review.

Thanks to everyone for your patience. I was out of office until the 4th but also became department chairperson at my university. So it took me a few days to catch up.

alberto-battistel commented 2 years ago

Hello @MariaPoliti, I apologize for my very slow response.

I am trying installing the package. I tried all proposed methods, but they all fail.

Installation via conda Installing the package via conda as suggested, but it failed. I opened an Issue with the description, see https://github.com/EISy-as-Py/hardy/issues/13#issue-1114920637.

Installation via GitHub repository I also tried following the GitHub repository, but is also failed. I opened another Issues with the details: https://github.com/EISy-as-Py/hardy/issues/14#issue-1114946713

Installation using evironment.yml Following https://hardy.readthedocs.io/en/latest/installation.html#installation-using-evironment-yml-recommended I tried the recommended way, but it also failed. I opened another Issue with the details: https://github.com/EISy-as-Py/hardy/issues/15#issue-1115195845

alberto-battistel commented 2 years ago

Dear @MariaPoliti https://github.com/MariaPoliti,

I have read the manuscript. I used the version it was send by .@wedon in the very beginning.

See in the attachment for a couple of comments I have for the text.

In  hardy/joss-paper/paper.md I see the .md file, but the captions for the figures are missing. This was a point to improve mentioned by @andrewtarzia https://github.com/andrewtarzia.

If you have a new version of the manuscript I will be happy to go through it directly.

General comments on the manuscript

The manuscript is well-written and clear, but some details and explanations are missing.

  1. Figure 1 should be explained more in detail.
  2. Figure 2 should also be explained more in detail. In particular, the Small Angle Scattering (SAS) data should be better described.
  3. It is not clear what is the RGB representation of the data.

I am going to flash a drive with a standard Ubuntu system and try the installation one more time.

On 16.01.22 16:38, George K. Thiruvathukal wrote:

@andrewtarzia https://github.com/andrewtarzia Thanks for your feedback. @MariaPoliti https://github.com/MariaPoliti Thanks for addressing @andrewtarzia https://github.com/andrewtarzia's feedback.

@alberto-battistel https://github.com/alberto-battistel Have you been able to review this submission? I don't see any input on the checklist or comments. We need input from you to complete this review.

Thanks to everyone for your patience. I was out of office until the 4th but also became department chairperson at my university. So it took me a few days to catch up.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3829#issuecomment-1013898172, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNDTJ26N6WDYSRHEJTFWV3UWLQ6RANCNFSM5GDZOXKQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

MariaPoliti commented 2 years ago

@whedon generate pdf

@alberto-battistel @andrewtarzia As mentioned in a previous message, the documentation has been updated and additional examples have been added- see doc/examples folder. If the documentation is not to a satisfactory level, we can add more examples/explanations.

@alberto-battistel the manuscript version on the repository is the final one. It seems like the captions are nto shown when opening the .md file on GitHub directly. If whedon does not generate a new version, you can generate it directly on this link: https://whedon.theoj.org/ by just pasting the repository's URL [ https://github.com/EISy-as-Py/hardy ].

Let us know if anything can be improved or need further attention.

Thank you all again for the feedback provided

whedon commented 2 years ago

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

alberto-battistel commented 2 years ago

Thank you, it is everything fine. On my side the article is as accepted. I am going to close or give the last comments to the issues I opened.

Best, Alberto

MariaPoliti commented 2 years ago

@gkthiruvathukal @andrewtarzia : Is there any update for this publication? When can we expect the review to be completed?

Thank you again all for your work and your feedback on this software!

andrewtarzia commented 2 years ago

I'll do it in the next few days. I wasn't sure it was ready yet, sorry!

gkthiruvathukal commented 2 years ago

@MariaPoliti I think we're almost ready to go.

Can you scroll up to the checklist of items remaining to be addressed by @andrewtarzia? It looks like you addressed all of these comments, but I did not see any follow up from @andrewtarzia. It may not be needed but I would like you to confirm that you addressed all of the points there.

Once I have said confirmation, I can proceed with final checks and to recommend acceptance.

andrewtarzia commented 2 years ago

Sorry about the delay. Having looked through the docs, examples and manuscript again, I am happy with it and recommend publication.

@gkthiruvathukal

MariaPoliti commented 2 years ago

@alberto-battistel There seems to be one task not checked off from your list. I believe we have addressed it, but I am just double checking with you.

@gkthiruvathukal What are the next steps after the review is completed? When can we expect for a final decision to be made about this publication?

Thank you agian all for your great feedback and your reviews

gkthiruvathukal commented 2 years ago

Hi @MariaPoliti, I'm sorry for the delay. I am ready to recommend acceptance. Stay tuned for the next steps.

gkthiruvathukal commented 2 years ago

@MariaPoliti, please do the following and check off once done:

Once these are done, I'll be able to recommend acceptance.

alberto-battistel commented 2 years ago

Sorry, I forgot to tick the last one.

gkthiruvathukal commented 2 years ago

Thanks, @alberto-battistel. @MariaPoliti, let me know when you have a chance to act on the above checklist of final steps.

MariaPoliti commented 2 years ago

@gkthiruvathukal Find the requested information below:

  1. Release name: HARDy v1.0.1 Release ; Tag: v1.0.1
  2. Zenodo Link: https://zenodo.org/record/6339767
  3. Zenodo DOI: 10.5281/zenodo.6339767

Let me know if I have missed something

gkthiruvathukal commented 2 years ago

@editorialbot set v1.0.1 as version

editorialbot commented 2 years ago

Done! version is now v1.0.1

gkthiruvathukal commented 2 years ago

@editorialbot set Zenodo DOI as archive

editorialbot commented 2 years ago

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

@editorialbot commands

gkthiruvathukal commented 2 years ago

@editorialbot commands

editorialbot commented 2 years ago

Hello @gkthiruvathukal, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Add to this issue's reviewers list
@editorialbot add @username as reviewer

# Remove from this issue's reviewers list
@editorialbot remove @username from reviewers

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Assign a user as the editor of this submission
@editorialbot assign @username as editor

# Remove the editor assigned to this submission
@editorialbot remove editor

# Remind an author or reviewer to return to a review after a 
# certain period of time (supported units days and weeks)
@editorialbot remind @reviewer in 2 weeks

# Check the references of the paper for missing DOIs
@editorialbot check references

# Perform checks on the repository
@editorialbot check repository

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for version
@editorialbot set v1.0.0 as version

# Set a value for archive
@editorialbot set 10.21105/zenodo.12345 as archive

# Set a value for branch
@editorialbot set joss-paper as branch

# Generates the pdf paper
@editorialbot generate pdf

# Recommends the submission for acceptance
@editorialbot recommend-accept

# Flag submission with questionable scope
@editorialbot query scope

# Get a link to the complete list of reviewers
@editorialbot list reviewers

# Open the review issue
@editorialbot start review
gkthiruvathukal commented 2 years ago

@editorialbot set 10.5281/zenodo.6339767 as archive

editorialbot commented 2 years ago

Done! Archive is now 10.5281/zenodo.6339767

gkthiruvathukal commented 2 years ago

@whedon generate pdf

editorialbot commented 2 years ago

My name is now @editorialbot

gkthiruvathukal commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

gkthiruvathukal commented 2 years ago

@editorialbot recommend-accept

editorialbot commented 2 years ago 
Attempting dry run of processing paper acceptance...
editorialbot commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1021/acs.chemmater.9b03043.s001 is OK
- 10.1002/aenm.201900555 is OK
- 10.1109/dsaa.2015.7344858 is OK
- 10.24963/ijcai.2017/352 is OK
- 10.1107/s1600576720000552 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/3037

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/3037, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

gkthiruvathukal commented 2 years ago

@MariaPoliti Thank you for your patience. @andrewtarzia and @alberto-battistel Thank you for your great help on reviewing this submission.

I have recommended acceptance and look forward to seeing this work published in JOSS.

alberto-battistel commented 2 years ago

Thanks, @alberto-battistel. @MariaPoliti, let me know when you have a chance to act on the above checklist of final steps.

I ticked the missing point. Is there anything else?

Kevin-Mattheus-Moerman commented 2 years ago

@MariaPoliti I have proofread your paper and it seems in order, but please do consider the following comment;

If you make changes to the paper please update it here using @editorialbot generate pdf. Thanks.

MariaPoliti commented 2 years ago

@editorialbot generate pdf

MariaPoliti commented 2 years ago

@Kevin-Mattheus-Moerman Thank you for pointing out the issues with capitalization. I have fixed those I could find in the manuscript and generated a new .pdf using the command you indicated.

kyleniemeyer commented 2 years ago

@editorialbot generate pdf