openjournals / joss-reviews

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

[REVIEW]: RMAVIS v1.0: a Shiny application for the analysis of vegetation survey data and assignment to GB NVC communities #6682

Closed editorialbot closed 1 month ago

editorialbot commented 5 months ago

Submitting author: !--author-handle-->@ZekeMarshall<!--end-author-handle-- (Zeke Marshall) Repository: https://github.com/NERC-CEH/RMAVIS Branch with paper.md (empty if default branch): main Version: v1.0 Editor: !--editor-->@mikemahoney218<!--end-editor-- Reviewers: @adithirgis, @rasanderson Archive: 10.5281/zenodo.12627165

Status

status

Status badge code:

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

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

@adithirgis & @rasanderson, 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 @mikemahoney218 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 @rasanderson

📝 Checklist for @adithirgis

editorialbot commented 5 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 5 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.06 s (1118.4 files/s, 323283.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
R                               51           3011           1140           8277
HTML                             2            548             12           3069
TeX                              4            131              0           1333
Markdown                         4            154              0            649
Rmd                              4            489            895            496
CSS                              1             39             23            114
YAML                             3             14              6             68
Dockerfile                       1              6              7             23
JSON                             1              0              0             19
-------------------------------------------------------------------------------
SUM:                            71           4392           2083          14048
-------------------------------------------------------------------------------

Commit count by author:

   184  ZekeMarshall
     1  Zeke Marshall
editorialbot commented 5 months ago

Paper file info:

📄 Wordcount for paper.md is 1179

✅ The paper includes a Statement of need section

editorialbot commented 5 months ago

License info:

🔴 License found: Creative Commons Attribution 4.0 International (Not OSI approved)

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

OK DOIs

- 10.1111/rec.13035 is OK
- 10.1007/BF00031691 is OK
- 10.1111/ecog.06547 is OK
- 10.1111/avsc.12516 is OK
- 10.33928/bib.2023.05.001 is OK
- 10.1046/j.1365-2664.2002.00718.x is OK
- 10.1007/s11258-020-01016-1 is OK
- 10.1017/9780521235587 is OK
- 10.1017/9780521391658 is OK
- 10.1017/9780521391665 is OK
- 10.1017/9781107340817 is OK
- 10.1017/CBO9780511541834 is OK
- 10.1023/a:1008948602316 is OK
- 10.5285/42C203C8-44DE-40E2-A694-B1E8CBD4C8E1 is OK
- 10.1002/ecs2.4726 is OK
- 10.5194/essd-7-203-2015 is OK

MISSING DOIs

- No DOI given, and none found for title: Ellenberg indicator F (moisture)
- No DOI given, and none found for title: Ellenberg indicator L (light)
- No DOI given, and none found for title: Ellenberg indicator N (nitrogen)
- No DOI given, and none found for title: Ellenberg indicator R (reaction)
- No DOI given, and none found for title: Ellenberg indicator S (salt tolerance)
- No DOI given, and none found for title: Taxon lists Botanical Society of Britain & Ireland
- No DOI given, and none found for title: Vegetation of the British Countryside - the Countr...
- No DOI given, and none found for title: UK Habitats Classification User Manual Version 1.1
- No DOI given, and none found for title: shiny: Web Application Framework for R
- No DOI given, and none found for title: Guidelines For Ecological Impact Assessment In The...
- No DOI given, and none found for title: BRYOATT - Attributes of British and Irish Mosses, ...
- No DOI given, and none found for title: Spreadsheet of Habitat Correspondences
- No DOI given, and none found for title: MATCH version 2
- No DOI given, and none found for title: TABLEFIT v. 3.0 & v.4, Programs for the Identifica...
- No DOI given, and none found for title: The Irish Vegetation Classification - an Overview ...
- No DOI given, and none found for title: ERICA - Engine for Relevés to Irish Communities As...
- No DOI given, and none found for title: S1 File
- No DOI given, and none found for title: National vegetation classification: Users’ handboo...
- No DOI given, and none found for title: MAVIS (Ver 1.03) User Manual
- No DOI given, and none found for title: A review of the National Vegetation Classification...
- No DOI given, and none found for title: Monitoring of forest removal and groundworks to re...
- No DOI given, and none found for title: An Exploration of Oceanic Wet Grasslands in the Sc...

INVALID DOIs

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

mikemahoney218 commented 5 months ago

👋🏼 @ZekeMarshall, @adithirgis, @rasanderson this is the review thread for the paper. Just about all of our communications will happen here from now on. :smile:

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 openjournals/joss-reviews#6615 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 4 weeks. Please let me know if you require some more time.

Please feel free to ping me (@mikemahoney218) if you have any questions/concerns.

rasanderson commented 5 months ago

Review checklist for @rasanderson

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

mikemahoney218 commented 5 months ago

@ZekeMarshall , I just noticed that you changed the license of the project a few days ago to a non open-source license. JOSS only reviews projects that are open source using an OSI-approved license (as mentioned on the first bullet point on the 'Submission requirements' checklist), which none of the creative commons licenses are (and CC recommends against using their licenses for software). Is there a reason you changed away from the MIT license? We will not be able to accept the project using the current (CC-BY) license.

ZekeMarshall commented 5 months ago

Hi @mikemahoney218 , apologies for this! I neglected to check that CCBY4.0 wasn't OSI-approved. I'll change this again in the morning when i'm in the office, likely to AAL, we just wanted to ensure attribution. PS thank you @adithirgis and @rasanderson for agreeing to review!

ZekeMarshall commented 5 months ago

Hi @mikemahoney218 , I've now changed the licence to the OSI-approved LGPL 3.0 and made a new release. Could you set the version to v0.9995? Thanks!

mikemahoney218 commented 5 months ago

Thanks @ZekeMarshall ! I believe we don't need to update the version until we accept the package -- feel free to bump it and release new ones whenever is convenient for you.

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

ZekeMarshall commented 5 months ago

Hi @rasanderson & @adithirgis, I've bumped the version to v0.9996. This version includes a few minor changes and one more substantial change, namely using the NBN to form the taxonomic backbone for vascular plants as well as bryophytes, so now there is only one source for the taxonomic backbone.

ZekeMarshall commented 5 months ago

Hi @rasanderson & @adithirgis, I've bumped the version to v0.9997. This version sets R 4.4 as the minimum version in response to the reported 'non-vulnerability' in R wherein arbitrary code can be executed when de-serialising R data objects, see here, which is not in fact fixed by R 4.4.0.

mikemahoney218 commented 4 months ago

Just as a quick note: I'm going to be traveling and generally less available until May 20th. I'll still be checking GitHub and email intermittently (so feel free to reach out with any questions or concerns), but apologies if it takes me a bit longer to respond than usual!

ZekeMarshall commented 4 months ago

Just as a quick note: I'm going to be traveling and generally less available until May 20th. I'll still be checking GitHub and email intermittently (so feel free to reach out with any questions or concerns), but apologies if it takes me a bit longer to respond than usual!

Thanks for letting us know and no worries at all. Have a good break!

rasanderson commented 4 months ago

Thanks for letting us know. I hope to have some first comments back fairly soon, so apologies for slight delay.

ZekeMarshall commented 4 months ago

@editorialbot set https://github.com/NERC-CEH/RMAVIS as repository

editorialbot commented 4 months ago

I'm sorry @ZekeMarshall, I'm afraid I can't do that. That's something only editors are allowed to do.

ZekeMarshall commented 4 months ago

Hi @mikemahoney218 , I've transferred the RMAVIS repository to our organisations Github (https://github.com/NERC-CEH/RMAVIS), would you be able to set this as the repository?

rasanderson commented 4 months ago

Hi @mikemahoney218 and @ZekeMarshall I've now had a chance to look through the code, paper, and Shiny app., and think this is a really nice piece of work. I had a PhD student a couple of years ago doing lots of work on pseudoquadrats and MAVIS classifications (see Butler, 2020 Chapter 3 ) so this is very interesting to read. There has been a need for this sort of app and functionality for some time. Some general points:

The Shiny app

This worked well, both the online site hosted (presumably) by CEH, and the version built from the R code from the GitHub site. A number of relatively minor points:

Paper

This provides a good summary of the needs for the work, is clearly written and correctly referenced. Main queries are:

R code and Github site

Again, relatively minor comments.

Enjoyed reading the research you've been doing - great stuff. Roy

mikemahoney218 commented 4 months ago

@editorialbot set https://github.com/NERC-CEH/RMAVIS as repository

editorialbot commented 4 months ago

Done! repository is now https://github.com/NERC-CEH/RMAVIS

mikemahoney218 commented 4 months ago

Thank you so much for your review, @rasanderson !

ZekeMarshall commented 4 months ago

Hi @rasanderson ,

Thank you for your positive review!

In response to your comments:

The Shiny App

Paper

R code and Github site

Thanks again, the changes I've made above are contained in the latest release (v0.9998)!

Best regards,

Zeke

ZekeMarshall commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

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

rasanderson commented 4 months ago

Hi @ZekeMarshall and @mikemahoney218 Many thanks for making the suggested changes, to the shiny app, article and GitHub landing page. Everything looks good to me, so I'm now happy to sign off and approve. Roy

mikemahoney218 commented 4 months ago

Thank you so much for your review, @rasanderson !

@adithirgis , I wanted to bump this now that we're about a month into the review window, and ask how your review is progressing/if you're still expecting to complete your review on the original timeline.

Just as a reminder, the first step in the review is to post @editorialbot generate my checklist as the start of a new comment in this thread, which will generate a review checklist for you to use!

ZekeMarshall commented 4 months ago

Hi @rasanderson, thank you very much! Your review is really appreciated. All the best, Zeke

mikemahoney218 commented 4 months ago

@adithirgis , I wanted to reach back out to ask how things are progressing, and to see if you had any questions/comments about your review for JOSS!

Just as a reminder, the first step in the review is to post @editorialbot generate my checklist as the start of a new comment in this thread, which will generate a review checklist for you to use.

Thanks!

adithirgis commented 3 months ago

Hi, apologies for my tardiness. Will review soon.

adithirgis commented 3 months ago

Review checklist for @adithirgis

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

adithirgis commented 3 months ago

@editorialbot generate pdf

editorialbot commented 3 months ago

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

adithirgis commented 3 months ago

Hello @ZekeMarshall, 

A great job on this software. A few minor points to share, please correct me if any points are already implemented -

Documentation

Functionality

Thank you, and apologies for the delay.

Adithi

ZekeMarshall commented 3 months ago

Hi @adithirgis ,

Thanks for your review!!

In response to your comments I have pushed a provisional v1.0 release to the v1 branch, this branch incorporates the following changes:

In terms of the package checks, ensuring there are no warnings is a tricky job. The biggest issue is the presence of non-ASCII strings in the taxonomic backbone data. I need to think about the best way to handle this as I need to maintain compatibility with the taxonomic backbone source and convert these non-ASCII strings to ASCII for RMAVIS and check they are all correct. I would also like to do this following a major update to the United Kingdom Species Inventory later this year. @mikemahoney218 are you happy for me to leave this job for a future release?

Lastly, @adithirgis the only comment I'm unclear about is in the Functionality section of your review - "Since this app can be used in other". What do you mean by this?

Thanks again! Please let me know if my changes have addressed your comments. When everyone is happy I will then release the actual v1.0.

Best regards,

Zeke

ZekeMarshall commented 3 months ago

Hi @mikemahoney218 and @adithirgis , just to note that I will be from the office next week, so may be slow to reply!

mikemahoney218 commented 3 months ago

@adithirgis , have you had a chance to look at @ZekeMarshall 's response?

ZekeMarshall commented 3 months ago

Hi @mikemahoney218 & @adithirgis , I'm back from annual leave now. Please let me know if there any other changes you'd like me to make @adithirgis !

adithirgis commented 3 months ago

Hello,

Thank you for your response. I see that the changes are implemented. Regarding - "Since this app can be used in other" I meant in other countries as well.

Last question now that we need install.packages("tinytex") is it a required package in DESCRIPTION? Apologies I am confused regarding this step. Rest everything looks good! Good luck with the package.

Thanks, Adithi

ZekeMarshall commented 3 months ago

Hi @adithirgis ,

Thanks for your reply and checking the v1 branch!

I've added {tinytex} in the suggests (not the imports) section of the DESCRIPTION file as the user may already have a LaTeX distribution installed and so may not need {tinytex}. Thanks for pointing this out!

@mikemahoney218 , if you are ok with the reviews from @adithirgis and @rasanderson am I now ok to take the following steps for potential acceptance?

  1. Merge v1 into main and release v1.0.0
  2. Re-generate the article pdf

Cheers,

Zeke

mikemahoney218 commented 3 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

mikemahoney218 commented 3 months ago

Thank you so much @rasanderson and @adithirgis for your thoughtful reviews here.

@ZekeMarshall, at this point could you follow the checklist in the above message (and yes, merge & release 1.0.0)? I can then move forward with recommending acceptance of the submission.

As a general note -- I'm generally offline this week for a holiday, so might be slow to respond to this issue. I'll be back next week and only intermittently online until then.

ZekeMarshall commented 3 months ago

@editorialbot generate pdf

editorialbot commented 3 months ago

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

ZekeMarshall commented 3 months ago

@editorialbot generate pdf

editorialbot commented 3 months ago

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