Joss-reviews: [REVIEW]: remBoot: An R package for Random Encounter Modelling

[whedon]

Submitting author: arcaravaggi (Anthony Caravaggi)
Repository: https://github.com/arcaravaggi/remBoot
Version: 0.0.0.9000
Editor: @karthik
Reviewer: @amoeba
Archive: 10.6084/m9.figshare.4684894

Status

Status badge code:

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

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) 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 questions

Conflict of interest

• [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: Does the release version given match the GitHub release (0.0.0.9000)?
• [x] Authorship: Has the submitting author (arcaravaggi) made major contributions to the software?

Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?
• [x] Functionality: Have the functional claims of the software been confirmed?
• [x] Performance: Have any performance claims of the software been confirmed?

Documentation

• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g. API method documentation)?
• [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [x] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @amoeba it looks like you're currently assigned as the reviewer for this paper :tada:.

: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 as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿

To fix this do the following two things:

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

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

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

@whedon commands

[amoeba]

I've had a short break to sit down with this submission. I will finish up tomorrow at some point.

Overall, things look pretty good and I only have minor suggestions so far. I think packages like this that implement methods from the literature are great and think this would be a useful package for others.

I've copied a subset of the checklist below to make comments on particular items in the checklist.

• Version: Does the release version given match the GitHub release (0.0.0.9000)?

No. There is no release(s) on GitHub. Please create a release for 0.0.0.9000.

• Authorship: Has the submitting author (arcaravaggi) made major contributions to the software?

Possibly and more-than-likely. The majority of the code was committed to the GitHub repository in one commit so the provenance of the code itself can't be verified. I checked this off as OK anyway.

• Installation: Does installation proceed as outlined in the documentation?

There is no documentation for how to install this software. Please include a section in the base README providing a complete description of how to install the software.

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

The statement given at the top of the README is sufficient but I suggest the reviewer add a sentence or two about why I want to use (1) this piece of software over others (are there others?) and (2) why I would want to use REM for my camera trap data. It looks like this copy is already in the included paper and vignette so this may be easy for the author to move into the README.

• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

The software includes one vignette which appears sufficient. I would encourage the author to (1) include a minimal worked example in the README and also fill in more details in the examples within function documentation (see my comment below).

• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g. API method documentation)?

  • Function documentation is missing argument types. Suggest adding them. For example, in remBoot, change

tm = camera hours

to

tm (numeric) The number of hours ...

• From reading over the vignette, it appears the core functionality of the package is made up of two functions: rem and remBoot. However, a number of other functions are exported. If these are not user-facing functions, I suggest not.

• Examples within function documentation is minimal. I suggest the reviewer spend some time expanding the examples in rem and remBoot so they are complete.

  • Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?

No tests are present. Given the simplicity of this package, it shouldn't take much in the way of tests to provide -- at a minimum -- a small set of regression tests.

• Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

No, please edit the README to include this information.

I'll move onto verifying the software tomorrow. Please reach out if you have any questions about my comments so far.

[amoeba]

I just had a chance to sit down to try to verify the functionality. Things seem in order but I have some comments about how REM is calculated and a lot of general comments about the code and help docs.

• Functionality: Have the functional claims of the software been confirmed?

  • The rem function appears to do what is outlined in the methods. I found that r and theta used in the calculation of REM are averaged which appears to be what is outlined in the methods:

  Means of r and θ across all relevant detections were used in a separate REM for each 1 km square.

  If this is the correct way to implement the REM method then the software should work correctly given sane inputs. However -- as a note -- the above quote implies the means are taken within a site rather than for the study as a whole. I think if the user calculates their REM densities using remBoot, they will be fine because remBoot splits by site. But if they calculate their REM densities directly with the rem function, they may need to be careful. Does this sound right @arcaravaggi?

  Also, the means for r and theta are calculated with na.rm =TRUE. What do NAs in these two values mean? I didn't find a place in the methods section of the linked papers about handling NAs. Maybe I just didn't look carefully enough.

  • The remBoot function appears to do what is outlined.

  Please comment on these two issues so I can check this item off.

General code comments:

• The user-facing functions perform little sanity-checking on input arguments and it seems like bad input might cause a result to be calculated that is non-sense (with no errors or warnings thrown). The JOSS review doesn't get into this level of detail on the code but I encourage the author to consider this if possible.
• It takes a bit of work to find out what columns the input data.frame should have. Given that the two user-facing functions are rem and remBoot, it would be great if each of these help pages included that information.
• The nboots argument is provided in the function signature of remBoot but never used in the function body. It appears the user can't configure this.
• The example code for split_dat includes the body of the code itself. A more useful example should be included if the user is supposed to use it.
• The function grpDat_func should be called something more useful to the user if it's intended to be a user-facing function. It's body should also not be defined in the example section.
• The function bsD mentions a function remsD which does not exist in the package. Something isn't right here. I see that remsD is a variable defined in the function body of remBoot. What's going on here?
• Help docs on most pages could be improved greatly. I can give more specific details if the author needs.
• Help doc on rem links to web and not to internal vignette. Seems like it's the same vignette that is included in the package and the local vignette would be vastly preferred to an online link to it.

That's quite a long list so I apologize in advance. I think I've covered enough topics for you to get started. So please let me know if I can help by answering questions or elaborating. If any of my suggestions seem difficult to implement, I might be able to assist in addressing them in a meaningful way (writing code, writing docs, etc).

[arcaravaggi]

Thanks for the detailed feedback, @amoeba. I'll do my very best to look at each issue in detail this week and will respond as soon as I can.

[arcaravaggi]

I've dealt with a number of the corrections and will be applying myself to the others (functionality clarification, tests, sanity checks) presently. Please let me know whether the corrections entered thus far are insufficient and/or inappropriate.

• Release 0.0.0.9000 created.
• Installation instructions added to the README.
• Moved package introduction text from vignette to the README.
• Included a brief, un-annotated example in the README.
• Corrected function documentation in remboot, described data requirements and expanded the example contained within.
• Corrected function documentation in rem, described data requirements and expanded the example contained within.
• Removed column 2 (legacy) from data and edited relevant files. This wasn't requested, but makes things a little neater as column 2 wasn't used.
• Edited Help docs for all user-facing functions.
• Added guidelines to README re: contributions and the reporting of errors.
• Corrected NAMESPACE to only export split_dat, rem and remboot.
• Added instruction re: _nboots_ to remBoot and examples.
• grpDat_func isn't user-facing and has been hidden.
• remSD is merely a vector used to store bootstrapping results: remsD <- lapply(grpDat, boot_sd)

[arcaravaggi]

Re: the external link to the vignette in rem. As I understand it, the html file needs to be parsed by http://htmlpreview.github.io/... for the output to be presented, as opposed to code. Is this not the case?

[amoeba]

Thanks for looking at these. I'll take a look over them soon, at least by this weekend.

Regarding vignettes, I'm not familiar enough with them off-hand but usually I can run vignette(package="remBoot") to get a list of the vignettes provided in remBoot and then, if I find a vignette called 'remBoot' in that list, I can the run vignette('remBoot', package='remBoot') to view a nicely-rendered vignette inside R (locally). I can look into this as it's something I should learn anyway.

[amoeba]

Thanks for making all of those changes. You'll notice I've checked off a number of the list items. Some minor details:

• Given the number of changes to the code, I suggest this submission get a new release of the package with the changes before finalizing this submission.

Documentation:

• The updated README is a good improvement. I'll sign off on this as-is but I suggest using code fences to provide rich markup for the code and also fix two broken references (Rowcliffe citation and Fig 2 reference)
• On the help doc for rem, the names in the function signature and the argument documentation are out of alignment (dat vs. .d.f)

Minor, not-crucial-for-review issues:

• I'm still a little perplexed about the nboots argument in remBoot. The way you've coded it, the argument nboots is never used and the value that is used by boot_sd is taken from the calling environment. I would expect this to be confusing to most users. Would you consider changing the way this works? If not, I suggest removing the nboots argument from the function and its docs and documentation the behavior elsewhere.

[arcaravaggi]

Thanks for the additional comments.

• Would you be happy for me to create a new release when we have completed the review process (prior to finalising)? If not, I can do it sooner, no problem.
• I've added code fences to README and fixed the Rowcliffe citation. I'm not sure what's wrong with the Fig. 2 reference, though.
• Re: nboots in remBoot, the parameter can either be defined in the calling environment or passed to remBoot with _tm_ and _v_. I have added a little text to the remBoot example, demonstrating this.

Regarding functionality (in your first response), users are able to calculate densities in two ways with rem: 1) by passing the dataframe to the function directly, thereby calculating densities across all data, or (2) by using grpDat for site-specific densities (assuming multiple sites). This is explained in the vignette.

Looking back at your comments, I think there are 3 issues outstanding (not including any carry-over from my responses, above):

• Add documentation on handling NAs
• Sanity checking (I'm looking into it of course, but is this essential for the purposes of the review?)
• Automated tests

Please do let me know if I've missed anything.

[amoeba]

Would you be happy for me to create a new release when we have completed the review process (prior to finalising)? If not, I can do it sooner, no problem.

Yes, doing just one release is ideal.

I'm not sure what's wrong with the Fig. 2 reference, though.

I expected there to be a figure 2 in the readme which there wasn't.

Your changes and responses are sufficient. Thanks for addressing those. I'm having a hard time actually running remBoot right now though:

> devtools::load_all(".")
> remBoot(hDat, tm = 1800, v = 0.89, nboots, error_stat = c("ci"))
Error in rem(bsDat, tm, v) : object 'tm' not found 

I haven't looked at the cause or at recent commits to see what's going on here but I thought I'd bring it to your attention.

Other than that, the automated tests are the only thing I think we need.

[arcaravaggi]

Hrm, that's interesting. I might be away for a few days, so will probably have to come back to this early next week. Sorry about that.

I've not written any automated tests before, so I'll have to do some reading. Do you have any recommendations on sources and/or implementation?

[amoeba]

A useful intro and guide is http://r-pkgs.had.co.nz/tests.html. A couple of tests for remBoot that test the return value for structure and correctness would probably be enough to check off the automated tests checklist item. It would catch the type of issue I received above (if in fact my issue above was not my fault, which it may be).

If you'd like, I can stub out testing and a few basic tests and push them your way.

[arcaravaggi]

That'd be much appreciated, thank you.

What would be the best way to acknowledge your efforts as reviewer, by the way? I usually mention reviewers in the acknowledgements of my papers, but JOSS is a bit different.

[amoeba]

I don't know, actually. Perhaps @karthik or someone at JOSS knows. I don't think there are any guidelines preventing you from putting acknowledgements in your paper.md so I'd probably put things there.

[arcaravaggi]

I've been unable to replicate your error this morning. I've re-downloaded the package and everything works on my end. Could you try reinstalling to see whether the error persists?

A quick note on nboots in remBoot: The function wasn't accepting the parameter if fed directly, so I've embedded the boot_sd function within remBoot and that seems to have fixed the issue. boot_sd is also still available as a standalone function for use with rem.

[amoeba]

Okay, maybe it was my fault. I'll double-check and get back to you. Thanks for the fix on the nboots param.

[amoeba]

You had previously said:

Corrected NAMESPACE to only export split_dat, rem and remboot

I noticed the roxygen tags didn't match with this so I removed extra @export tags. Then I noticed boot_sd is used in the vignette so I exported boot_sd.

I tweaked this stuff and also ran and re-ran devtools::check and devtools::document until things ran smoothly (fixing issues along the way). See my PR. Unfortunately, it looks like RStudio clobbered your line endings, making the git diff less than useful.

Hopefully my set of documentation changes are agreeable and my example tests are useful. I'll review the checklist once some or all of my changes are merged.

[arcaravaggi]

I left the export tags on the other functions as they didn't work locally without them, for some reason. If they work now, that's great, thanks.

RStudio really has done a number on my lines. I'll comb through and try to identify the changes.

[amoeba]

Yeah I can re-PR with a non-borked version if you like. Are you on Windows? I have my RStudio set up to save UNIX-style line endings but it appears I can change them to Windows-style.

[arcaravaggi]

I am, yes. Sorry about that.

[amoeba]

I was unable to get my edits with RStudio to produce meaningful diffs so I made an issue and pasted what I was finding: https://github.com/arcaravaggi/remBoot/issues/17. Could you please take a look and look into fixing those issues?

[amoeba]

@arcaravaggi and I worked from https://github.com/arcaravaggi/remBoot/issues/17 to address my remaining issues. I have now checked the automated tests list item as I wrote up a minimal set of regression tests. This package isn't particularly involved so a much larger test suite might not be strictly necessary.

I change my review to Accept. We need a new release of the R package to accompany the JOSS review at which point I think everything is done here.

[arcaravaggi]

I have created a new release to accompany the JOSS review: https://github.com/arcaravaggi/remBoot/releases/tag/v0.1.0

[arcaravaggi]

I have created a new figshare archive as required by JOSS: https://doi.org/10.6084/m9.figshare.4684894

Once the JOSS article is published and a DOI has been assigned, I will update the package's README with citation information, include a CITATION file, and add the JOSS article DOI to the figshare archive in the form of a comment.

It is likely that there will be developments in Random Encounter Modelling from camera trap data in the future. @arfon, are there any limits on continued development of packages which have been accepted for publication in JOSS?

[arcaravaggi]

@amoeba What happens with the submission, now?

[amoeba]

@whedon commands

[whedon]

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

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

# List the GitHub usernames of the JOSS editors
@whedon list editors

# List of JOSS 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

# Open the review issue
@whedon start review

:construction: Important :construction:

This is all quite new. Please make sure you check the top of the issue after running a @whedon command (you might also need to refresh the page to see the issue update).

[arfon]

I think next steps are on me now to accept this submission.

@arfon, are there any limits on continued development of packages which have been accepted for publication in JOSS?

@arcaravaggi - nope. This is totally fine 😁

[amoeba]

Thanks @arfon!

[arcaravaggi]

Thanks, @arfon. Sorry if I appeared impatient - I was just unsure. Thanks for the clarification, too. :)

[arfon]

@whedon assign 10.6084/m9.figshare.4684894 as archive

[arfon]

@whedon set 10.6084/m9.figshare.4684894 as archive

[whedon]

OK. 10.6084/m9.figshare.4684894 is the archive.

[arfon]

@arcaravaggi - could you please move your references into a paper.bib file and cite them inline from the paper.md markdown file? We compile the paper using pandoc which handles the production of the PDF etc.

[arcaravaggi]

I've created the paper.bib file and edited the paper.md file to include inline citations. I've retained the 'References' header in paper.md after following guidelines on the RStudio page.

[arfon]

@amoeba many thanks for reviewing this submission ✨

@arcaravaggi - your paper is now accepted into JOSS and your paper DOI is http://dx.doi.org/10.21105/joss.00176 :zap: 🚀 💥

[arcaravaggi]

Thanks, @arfon. Thanks for all your efforts, @amoeba. ✨

[amoeba]

It was a pleasure working with you @arcaravaggi ! Congratulations.

[karthik]

Sorry I was away on vacation when the last few discussions happened and there was no way to leave a vacation notice.

@amoeba It is possible to add @arcaravaggi as a reviewer in your package DESCRIPTION using that role. This is still possible after acceptance. You can do it in your next CRAN update.