openjournals / joss-reviews

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

[REVIEW]: OpenRepGrid.ic: A software for Interpretive Clustering #3292

Closed whedon closed 1 year ago

whedon commented 3 years ago

Submitting author: !--author-handle-->@markheckmann<!--end-author-handle-- (Mark Heckmann) Repository: https://github.com/markheckmann/OpenRepGrid.ic Branch with paper.md (empty if default branch): joss Version: v0.6.1_JOSS_submission Editor: !--editor-->@gkthiruvathukal<!--end-editor-- Reviewers: @aj2duncan, @brunaw Archive: 10.5281/zenodo.7655618

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

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

@aj2duncan & @brunaw, 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 @aj2duncan

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @brunaw

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. @aj2duncan, @brunaw 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
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.04 s (453.2 files/s, 82699.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
R                               10            366            327           1661
HTML                             1              3              0            553
Markdown                         3             83              0            143
Rmd                              2             73            136             56
YAML                             3             17              8             41
-------------------------------------------------------------------------------
SUM:                            19            542            471           2454
-------------------------------------------------------------------------------

Statistical information for the repository 'b59425c4d12ec552f5c00de5' was
gathered on 2021/05/19.
No commited files with the specified extensions were found.
whedon commented 3 years ago

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

 Can't find any papers to compile :-(
markheckmann commented 3 years ago

@whedon generate pdf from branch joss

whedon commented 3 years ago
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
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:

brunaw commented 3 years ago

Dear @markheckmann,

Here are a few of my initial comments/notes about the paper and code. Could you review and give any answers/solutions for what I've pointed out? After that, I'll do my review checklist for your submission:

I understand that those might be just helper functions, but ideally, they should be properly documented and organized like the rest of the code

Thank you,

Bruna

whedon commented 3 years ago

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

whedon commented 3 years ago

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

aj2duncan commented 3 years ago

Apologies @gkthiruvathukal , I'm now ready to start the review but appear to have missed the timings on the invitation. Could it possibly be sent again? Sorry.

aj2duncan commented 3 years ago

Sorry (again) @gkthiruvathukal I am still unable to edit the checklist. No rush as I'm happy to proceed with the review and catch up on the checklist later.

danielskatz commented 3 years ago

@whedon re-invite @aj2duncan as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

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

danielskatz commented 3 years ago

👋 @aj2duncan - ☝️ this should enable you to check the boxes...

markheckmann commented 3 years ago

@aj2duncan : I fixed the issue you posted on github. There were some breaking changes in a package dep and it should work again now. Thanks!

aj2duncan commented 3 years ago

@markheckmann, great - thanks. I'll get back to testing the app. @brunaw, has made some queries/comments☝️, have you had a chance to look at any of them?

Thanks again for fixing the app. I'll get back to you as soon as I can.

markheckmann commented 3 years ago

Dear @brunaw,

thanks for your comments and apologies for the late reply! I inserted my answers to your comments below. There is a new version of the manuscript available now in which I made the changes accordingly.

Thanks for your time and effort!

Best wishes Mark


I am a little bit confused about whether the focus of the paper is on the shiny app or the IC implementation as an R package. Is the IC implementation new and the shiny app is there for demonstration purposes only? I think that's happening because everything is being described as the 'software for IC'

The paper is on the shiny app. It is the only software which implements the IC method. The software is necessary as the steps of the IC procedure are too laborious to be done by hand. So researchers will need a software to do it. We tried to explain this in the statement of need section. Please let me know if this is still too unclear and if so, could you maybe indicate which parts you find confusing?

In the "Repertory Grid Technique" section, the transition between talking about the data and the results shown in the plot could be smoother (lines 36 and 37). The text seems to assume that the reader is familiar with what is usually plotted in Figure 1

I rearranged and slightly extended the section to make it more readable and give the reader a better understandiong what he/she sees in Figure 1. The paragraph is supposed to give readers who are unfamiliar with repertory grids a rough idea about the resulting data structure. A more thorough introduction to the RGT is, however, beyond the scope of this paper, I think.

The figures have two 'Figure x' labels

Thanks, I corrected this.

I recommend not using terms such as "is shown below" and use the figure numbers instead

Thanks. I changed this in the latest version of the manuscript.

The https://ic.openprepgrid.org/ URL couldn't be reached here (as it is in the paper), but http://ic.openrepgrid.org/ works correctly

I changed the url to http. The app is currently hosted on a free tier shinyapps.io account, where SSL is not supported. I plan to move it to my own server with SSL support in the future. Then both protocols (http and https) will be supported, so http seems to be the best choice for the manuscript.

Likewise, the docker image needs a user login. Is that the standard for all Docker users or only allowed users can see the app?

The docker image is public, i.e. anyone can pull and use it without authentication. Assuming you have the docker deamon installed and running, just type docker pull markheckmann/openrepgrid.ic into the console/terminal and it should pull the latest image from dockerhub. I also added instructions on how to run the image locally under the dockerhub link in the manuscript.

The shiny app won't open locally to me due to functions not being correctly loaded: Error in rightSidebar: could not find function "rightSidebar"

This is the same issue @aj2duncan had and due to breaking changes in a package dep. I fixed it in release v0.5.1.

Not all functions are fully documented, for example: count_matches() , replace_one() I understand that those might be just helper functions, but ideally, they should be properly documented and organized like the rest of the code

All functions, including all helpers, are documented know.

There are a few 'random' lines of comments in the R files (e.g. 02-calculate.R file, line 142)

I cleaned up the code. There are a few commented out lines of R code which I want to keep where I added an extra comment why they are there.

markheckmann commented 3 years ago

@whedon generate pdf from branch joss

whedon commented 3 years ago
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
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:

aj2duncan commented 3 years ago

Hi @markheckmann,

My apologies for the delay in getting back to this review. I'm happy with the paper and I think there are just a few minor things for you to address before I can complete the review.

Please let me know if you've got any queries or comments about any of this.

Thanks,

Andrew

gkthiruvathukal commented 3 years ago

@whedon commands

whedon commented 3 years ago

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Re-invite a reviewer (if they can't update checklists)
@whedon re-invite @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version

# Open the review issue
@whedon start review

EDITORIAL TASKS

# All commands can be run on a non-default branch, to do this pass a custom 
# branch name by following the command with `from branch custom-branch-name`.
# For example:

# Compile the paper
@whedon generate pdf

# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name

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

# Ask Whedon to do a dry run of accepting the paper and depositing with Crossref
@whedon recommend-accept

# Ask Whedon to check the references for missing DOIs
@whedon check references

# Ask Whedon to check repository statistics for the submitted software
@whedon check repository

EiC TASKS

# Invite an editor to edit a submission (sending them an email)
@whedon invite @editor as editor

# Reject a paper
@whedon reject

# Withdraw a paper
@whedon withdraw

# Ask Whedon to actually accept the paper and deposit with Crossref
@whedon accept deposit=true
gkthiruvathukal commented 3 years ago

@markheckmann Is there any update? See @aj2duncan's comment above.

markheckmann commented 3 years ago

Hi @gkthiruvathukal , sorry for my delay. I have not found the time yet to make the changes as suggested by @aj2duncan. Thanks again @aj2duncan for your comments! Things will slow down here in 1,5 weeks and iI will submit a new version by the end of October. I hope that this is okay for everyone. Best wishes Mark

gkthiruvathukal commented 3 years ago

@markheckmann This is fine. We'll look for your follow-up by the end of October.

markheckmann commented 3 years ago

Hi, I need some more time and will upload the changes this coming weekend. Bests Mark

gkthiruvathukal commented 3 years ago

Hi @markheckmann. No problem. Please take your time.

gkthiruvathukal commented 2 years ago

@markheckmann Can you please provide an update on when you'll be able to make the changes?

@brunaw Have you also had a chance to review the submission? I guess with the significant changes required, it may make sense to wait until @markheckmann has made the changes proposed by @aj2duncan.

markheckmann commented 2 years ago

Dear @gkthiruvathukal , thanks for following up on this! I will definitely be able to make the changes by the end of February.

gkthiruvathukal commented 2 years ago

@markheckmann It's amazing how February always seems to come and go without a warning. Can you let us know how things are going with your changes? If you need more time, no problem!

markheckmann commented 2 years ago

@gkthiruvathukal: well, what can I say 🙈🙈🙈. I have found time to implement some of the changes. But especially the request to add an interactive analysis output (not only the static Excel analysis to download) will require some more time. I have two weeks of holiday in April, in which I am confident to find enough time to finish the project. So yes, more time would be appreciated.

gkthiruvathukal commented 2 years ago

@markheckmann Can you give me an update? I'd like to move forward on acceptance soon. Having Excel download does not sound difficult. At the same time, it does not seem "mission-critical" either. If you have acted upon all other suggestions, I think we could move forward here. Can you let me know?

markheckmann commented 2 years ago

@gkthiruvathukal I found some time to work on it and will finish the changes in the next 1 or 2 days, so we can move ahead.

gkthiruvathukal commented 2 years ago

Thanks for the update, @markheckmann!

markheckmann commented 2 years ago

Dear all,

thanks for your patience and my apologies with regard to the timeliness of our revisions.

@aj2duncan: please find below our answers to your comments:

Can you confirm which of the OSI approved licenses you are using?

The software uses the MIT license. The license statement is now contained in the package’s DESCRIPTION file. A copy of the MIT license itself is additionally found in the root folder of the R package repo.

The documentation in the repository/app is missing some of the detail that is in your paper. This includes the statement of need, functionality documentation and community guidelines. I think that the detail in the paper is fine but could be completely or partially replicated in the repo.

As you suggested, we now replicated the text from the paper (with minor modifications only) directly in the README of the repo.

Most of the functionality is documented in the app itself but it took me a minute to work out that "The results of the IC analysis are not displayed interactively but are included in an MS Excel file that can be generated and downloaded." as stated in the paper. It is implied by the popup in the app itself but most shiny users would probably expect the graph to be drawn in the app itself.

We now added a new info box at the top of the analysis tab of the software explaining that the final result of the analysis is a downloadable file, so the user will directly see it. We also discussed adding interactivity directly in the tool, but came to the conclusion, that the benefits would be minor. After the quantitative analysis, a researcher must thoroughly look at the results first and make a qualitative interpretation. This process takes quite some time and iterations are rather slow, so we decided against the interactive display of results.

The step of loading library(OpenRepGrid.ic) before running ic() is missing from the repo README. As above, it is present in the paper.

We now added the line to the repo README.md

@brunaw: We already answered your comments above.

Further changes compared to the last version:

image

Please let me know if you have any further comments / see need for further changes or if we are ready to move ahead. The package version 0.6 will hit CRAN in a few days if you feel comfortable with the current status.

Thanks again! Mark

editorialbot commented 2 years ago

My name is now @editorialbot

markheckmann commented 2 years ago

@whedon generate pdf from branch joss

editorialbot commented 2 years ago

My name is now @editorialbot

markheckmann commented 2 years ago

@editorialbot set joss as branch

editorialbot commented 2 years ago

Done! branch is now joss

markheckmann 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:

aj2duncan commented 2 years ago

Hi @markheckmann, thanks for this. I will try to check them in detail over the weekend and get back to you.

aj2duncan commented 2 years ago

Hi @markheckmann,

Thanks for making those alterations. I think everything is good to go @gkthiruvathukal. Please let me know if you need anything else from me.

All the best.

Andrew

gkthiruvathukal commented 2 years ago

@aj2duncan Thank you for your feedback. For me to move forward (with acceptance), I need @brunaw to sign off and complete their checklist atop this issue thread. Once @brunaw completes the checklist and follows up here, we should be good to go. From what I can tell, you have done everything asked of you, @markheckmann!

gkthiruvathukal commented 2 years ago

@brunaw Can you please let us know whether you plan to complete your review and the checklist? I would really like to proceed with accepting this submission (soon).

brunaw commented 2 years ago

@gkthiruvathukal I can finish it by this Thursday, if that works?

gkthiruvathukal commented 2 years ago

@brunaw it works perfectly! Thank you!

brunaw commented 2 years ago

Doing it now, I should upload the review soon!