Joss-reviews: [REVIEW]: filesstrings: An R package for file and string manipulation

[whedon]

Submitting author: @rorynolan (Rory Nolan)
Repository: https://github.com/rorynolan/filesstrings
Version: v0.4.1
Editor: @karthik
Reviewer: @benmarwick
Archive: 10.5281/zenodo.801452

Status

Status badge code:

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

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 (v0.4.1)?
• [x] Authorship: Has the submitting author (@rorynolan) 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. @benmarwick 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

[benmarwick]

Review of filesstrings

I'm using the ropensci reviewer template here:

• [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work

y: yes
i: could be improved
n: no

General checks

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

Documentation

The package includes all the following forms of documentation:

• [i] A statement of need clearly stating problems the software is designed to solve and its target audience in README
• [i] Installation instructions: for the development version of package and any non-standard dependencies in README
• [i] Vignette(s) demonstrating major functionality that runs successfully locally
• [y] Function Documentation: for all exported functions in R help
• [y] Examples for all exported functions in R Help that run successfully locally
• [i] Community guidelines including contribution guidelines in the README or CONTRIBUTING, and URL, Maintainer and BugReports fields in DESCRIPTION

Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

• [i] A short summary describing the high-level functionality of the software
• [y] Authors: A list of authors with their affiliations
• [i] A statement of need clearly stating problems the software is designed to solve and its target audience.
• [n] References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

• [y] Installation: Installation succeeds as documented.
• [y] Functionality: Any functional claims of the software been confirmed.
• [y] Performance: Any performance claims of the software been confirmed.
• [y] Automated tests: Unit tests cover essential functions of the package
  and a reasonable range of inputs and conditions. All tests pass on the local machine.
• [y] Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

• [ ] The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 3

---

Review Comments

This is an expertly-written R package that adheres to many conventions of good practice and efficient programming. It contributes some useful functions in R for file, directory, and string manipulation that could be very useful for many users.

The statement of need should be more specific, I recommend something like: "This package provides convenient functions for moving files, deleting directories, removing spaces from file names, and a variety of string operations that I wished were easier in base R."

Installation instructions should show how to install dev version, e.g. "To install the development version , run:
devtools::install_github("rorynolan/filesstrings")"

Vignettes are present. The main issue with the vignettes is that they are not very useful, especially the Files vignette. The heading in both vignettes, "Please Read the Manual", is a bit puzzling - the reader is already reading the manual (i.e. the vignette), so why ask them to do this? The examples in the Files vignette are too artificial, and don't represent a typical user's experience. I recommend to consult vignettes some widely used packages and re-write to match their style and accessibility for the typical user. I've edited the readme to indicate text that is more user-friendly.

Recommend to add contribution guidelines in the README or a CONTRIBUTING file, with a link from the readme

In paper.md and elsewhere, 'cleverly' doesn't seem like the right way to describe file moving in base R. 'Unintuitive' seems more appropriate. There are no references, which is unusual. Recommend to take a look at R packages that have been accepted by JOSS to see what they put as references.

I will make a pull request to help with some of these suggestions.

Now I'm using the ropensci pkg guidelines here:

Package naming

filesstrings with the ss in the middle seems a bit awkward. Why not filestrings?

Function/variable naming & general syntax

ropensci recommends snake_case, and the base functions that this pkg complements use period.separated. But in this pkg we have UpperCamelCase, which is unexpected, and will likely make it harder for the user to recall the function names (it's one of the least common naming conventions)

It seems more natural and easier to remember if MoveFiles is named move.files. Similarly, RemoveDirs should be dir.delete to echo dir.create.

It seems more nature for PutFilesInDir to be a default behaviour of MoveFiles when the directory name doesn't exit

README

✔ The readme is good, I've made some suggestions to make the text more user-friendly

Code of Conduct

Recommend to add, cf. the ropensci guidelines:

• We recommend that you use a code of conduct such as the Contributor Covenant in developing your package. You can document your code of conduct in a CODE_OF_CONDUCT.md or CONDUCT.md file in the package root directory, and linking to this file from the README.md file. devtools::use_code_of_conduct() will add the Contributor Covenant template to your package.

Documentation

✔ Uses roxygen2

News

✔ There is a NEWS.md

Recommend to use sections as needed, including: NEW FEATURES, MINOR IMPROVEMENTS,
BUG FIXES, DEPRECATED AND DEFUNCT. Under each header list items as needed. For each item give
a description of the new feature, improvement, bug fix, or deprecated function/feature. Link
to any related GitHub issue like (#12). The (#12) will resolve on GitHub in Releases to a
link to that issue in the repo.

Authorship

✔ The DESCRIPTION file does list the package authors and contributors to a package, using the Authors@R syntax to indicate their roles (author/creator/contributor etc.)

Testing

✔✔ There is an extensive set of tests, using testthat

Versioning

✔ Uses semantic versioning.

✔ Uses Git tags

Continuous integration

✔ Uses continuous integration.

Examples

✔ Includes extensive examples in the documentation.

Package dependencies

✔ Uses Imports instead of Depends

Console messages

Recommend to add some message()s to confirm to the user that a function completed successfully. For example, something like "37 files moved, no errors"

[karthik]

👋 Hi @rorynolan, Can you please respond to Ben's comments and update this thread please?

[rorynolan]

Dear @benmarwick ,
Thank you for taking the time to review my package. I agree with all of your points and will amend my package accordingly. I'll have it ready in a few weeks.
Regarding the renaming of the package, since it's already on CRAN, I think I'll leave the name as it is, even though filestrings would be better for sure.
Regarding the snake_case naming of functions, I made the package according to google's R style guide. I now use snake_case in everything I do, but when I switched styles I was knee-deep in this package. It already has over a thousand downloads on CRAN, so do you think it would do more good than harm for me to restyle it? If you think it's a good idea, I'm willing to.
Thanks again,
Rory

[karthik]

@rorynolan Great! Please at mention me when you're ready for us. thanks!

[rorynolan]

Hi @benmarwick
Apart from renaming the package and changing the style, I've implemented everything you said and pushed the changes to github.
In particular, I've

• tried to improve the vignettes
• structured the NEWS file as suggested
• added function names file.move() and dir.remove() to comply with base R's naming style
• changed the paper and README to include a better statement of purpose
• added instructions to README to include how to install dev version
• made PutFilesInDir() to be a default behaviour of MoveFiles()
• added references to the paper (my bibtex references weren't integrating properly into paper.md so I put them in explicitly)
• added a CONTRIBUTING file and mentioned it in my README
• added the informative messages

Let me know what you think regarding the restyling and the changes I've made.
No rush, I'm not in a hurry to publish.
Thanks again again,
Rory

[benmarwick]

Thanks, I've reviewed the changes made at https://github.com/rorynolan/filesstrings/commit/dc5338deb72af5c7c3489e275efb3b6b9ec0e3bf, and they are excellent. I'm delighted to say that the author has responded to my review and made changes to my satisfaction. I recommend approving this package. Congratulations on such high quality work on a very useful package!

[arfon]

@rorynolan - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

[rorynolan]

Thanks @benmarwick for the review and kind words.
Thanks @karthik for editing.
Thanks @arfon for the great JOSS service. DOI: 10.5281/zenodo.801452

[arfon]

@whedon set 10.5281/zenodo.801452 as archive

[whedon]

OK. 10.5281/zenodo.801452 is the archive.

[arfon]

@rorynolan - Could you move the references you currently have in the paper.md file into a paper.bib file and cite them directly please? (You can read how to do that here). The citations in paper.bib are automatically compiled into the final PDF by Pandoc and so they don't need to be listed at the end of the paper.md file.

[karthik]

👍

[rorynolan]

Hi @arfon,
This is done now.
Thanks again,
Rory

[arfon]

@benmarwick - many thanks for your review here and to @karthik for editing this one ✨

@rorynolan - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00260 ⚡️:rocket: :boom:

[rorynolan]

Thanks again everyone. JOSS has been a fantastic experience, I'll be recommending it to all my colleagues.