rtimicropem

[maelle]

Summary

• What does this package do? (explain in 50 words or less):

This package aims at supporting the analysis of PM2.5 measures made with RTI MicroPEM. RTI MicroPEM are personal monitoring devices (PM2.5 and PM10) developped by RTI international.

• Paste the full DESCRIPTION file inside a code block below:

Package: rtimicropem
Type: Package
Title: Supports the Analysis of RTI MicroPEM Output Files
Version: 1.3
Authors@R: c(person("Maëlle", "Salmon", email = "maelle.salmon@yahoo.se", role = c("aut", "cre")),
             person("Zheng", "Zhou", comment = "Department of Environmental Health Sciences Mailman School of Public Health at Columbia University, New York, USA", role = c("aut")),
             person(family = "ERC Grant Agreement number 336167 - the CHAI Project", role = c("fnd")),
             person("Carles", "Milà", role = c("ctb")),
             person("Sreekanth", "Vakacherla", role = c("ctb")),
             person("Cathryn", "Tonne", role = c("ctb")),
             person("Julian", "Marshall", role = c("ctb")))
Description: Supports the input and reproducible analysis of RTI MicroPEM output files.
License: GPL (>= 2)
LazyData: TRUE
RoxygenNote: 6.0.1
Suggests: testthat,
    rmarkdown,
    xtable,
    shiny
Imports: lubridate,
   ggplot2,
   dplyr (>= 0.4.3),
   rbokeh,
    R6,
   changepoint,
   tibble,
    tidyr,
    lazyeval,
    readr,
    pathological,
    stringr,
    knitr,
    methods
Depends: R (>= 3.3.1)
VignetteBuilder: knitr
URL: http://github.com/masalmon/rtimicropem
BugReports: http://github.com/masalmon/rtimicropem/issues
Encoding: UTF-8

• URL for the package (the development repository, not a stylized html page):

https://github.com/masalmon/rtimicropem

• Please indicate which category or categories from our package fit policies this package falls under *and why(? (e.g., data retrieval, reproducibility. If you are unsure, we suggest you make a pre-submission inquiry.):

[e.g., "data extraction, because the package parses a scientific data file format"]

This package fits in the data extraction category because it allows to transform files produced by the RTI MicroPEM, which are scientific devices, into data.frames.

• Who is the target audience?

Scientists using RTI MicroPEMs for collecting personal PM2.5 data.

• Are there other R packages that accomplish the same thing? If so, how does
  yours differ or meet our criteria for best-in-category?

Not to my knowledge.

Requirements

Confirm each of the following by checking the box. This package:

• does not violate the Terms of Service of any service it interacts with.
• has a CRAN and OSI accepted license.
• contains a README with instructions for installing the development version.
• includes documentation with examples for all functions.
• contains a vignette with examples of its essential functions and uses.
• has a test suite.
• has continuous integration, including reporting of test coverage, using services such as Travis CI, Coeveralls and/or CodeCov.
• I agree to abide by ROpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

Publication options

• Do you intend for this package to go on CRAN?
• Do you wish to automatically submit to the Journal of Open Source Software? If so:
  • The package contains a paper.md with a high-level description in the package root or in inst/.
  • The package is deposited in a long-term repository with the DOI:
  • (Do not submit your package separately to JOSS)

Detail

• Does R CMD check (or devtools::check()) succeed? Paste and describe any errors or warnings:

• Does the package conform to rOpenSci packaging guidelines? Please describe any exceptions:

• If this is a resubmission following rejection, please explain the change in circumstances:

• If possible, please provide recommendations of reviewers - those with experience with similar packages and/or likely users of your package - and their GitHub user names:

[sckott]

Editor checks:

• Fit: The package meets criteria for fit and overlap
• Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
• License: The package has a CRAN or OSI accepted license
• Repository: The repository link resolves correctly
• Archive (JOSS only, may be post-review): The repository DOI resolves correctly
• Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

---

Editor comments

Thanks for your submission @maelle !

Currently seeking reviewers. It's a good fit and not overlapping.

• goodpractice::gp() output is below. Looks quite clean.

It is good practice to

  ✖ write unit tests for all functions, and all package code
    in general. 92% of code lines are covered by test cases.

    R/clean_measures.R:56:NA
    R/clean_measures.R:74:NA
    R/clean_measures.R:75:NA
    R/clean_measures.R:76:NA
    R/clean_measures.R:77:NA
    ... and 26 more lines

  ✖ avoid long code lines, it is bad for readability. Also,
    many people prefer editor windows that are about 80 characters
    wide. Try make your lines shorter than 80 characters

    inst/shiny-examples/myapp/server.R:50:1
    inst/shiny-examples/myapp/ui.R:42:1
    R/identify_lags.R:20:1
    R/identify_lags.R:25:1
    R/micropem_class.R:236:1

  ✖ not import packages as a whole, as this can cause name
    clashes between the imported packages. Instead, import only the
    specific functions you need.
  ✖ fix this R CMD check NOTE: Namespace in Imports field not
    imported from: ‘R6’ All declared Imports should be used.

---

Reviewers: @LucyMcGowan @karawoo
Due date: 2017-07-14

[maelle]

Thanks a lot @sckott , will take a look in particular at the Imports soon.

[maelle]

@sckott The package no longer imports ggplot2 as a whole but I don't understand where the R CMD check NOTE comes from.

fix this R CMD check NOTE: Namespace in Imports field not imported from: ‘R6’ All declared Imports should be used.

In micropem-class.R I actually import R6::R6Class 🤔

[sckott]

Right - i get that error sometimes too and doesn’t seem to make sense - will look into that warning message in general cause I'd like to know

[sckott]

@maelle do you have a travis or other build to reference with that note?

[maelle]

Will check tomorrow, Thanks!

[sckott]

ok, thanks

[maelle]

• I apparently don't get that note on Travis, and I didn't get it on my PC, probably linked to the R CMD check options that goodpractice applies? The only NOTE I get is checking DESCRIPTION meta-information ... NOTE Authors@R field gives persons with no valid roles: ERC Grant Agreement number 336167 - the CHAI Project [fnd] which was expected, and the output of goodpractice::gp() is

── GP rtimicropem ──────────────────────────────────────────────────────────────────────────────

It is good practice to

  ✖ write unit tests for all functions, and all package code in general. 92% of code
    lines are covered by test cases.

    R/clean_measures.R:56:NA
    R/clean_measures.R:74:NA
    R/clean_measures.R:75:NA
    R/clean_measures.R:76:NA
    R/clean_measures.R:77:NA
    ... and 26 more lines

  ✖ avoid long code lines, it is bad for readability. Also, many people prefer editor
    windows that are about 80 characters wide. Try make your lines shorter than 80
    characters

    inst\shiny-examples\myapp\server.R:50:1
    inst\shiny-examples\myapp\ui.R:42:1
    R\identify_lags.R:20:1
    R\identify_lags.R:25:1
    R\micropem_class.R:235:1

────────────────────────────────────────────────────────────────────────────────────────────────

And it seems I'm using goodpractice latest version, weird.

• The build on Mac R-devel on Travis fails but I wasn't too worried about it, it looked like one of these dependencies issues that get fixed over time... Maybe I should actually worry? 🤔

[sckott]

talked to some people about the warning about Imports and they said to just import the package, either all of it, or importFrom

hmm, little experience with osx builds, not sure

[sckott]

reviewers assigned @LucyMcGowan @karawoo - due date up above

[LucyMcGowan]

Package Review

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

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

Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

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

Functionality

• Installation: Installation succeeds as documented.
• Functionality: Any functional claims of the software been confirmed.
• Performance: Any performance claims of the software been confirmed.
• 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.
• 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: 2.5

---

Review Comments

🙌 🎉 💯

This is an excellent package for streamlining the analysis of PM2.5 measures made with RTI MicroPEM. It seems this would be very useful for labs utilizing these methods. I have a few small comments below, but overall big 👍!

Tests

• The tests ran successfully on OS X Sierra 3.3.3
• All code in the README, vignette, and examples ran as expected.
• I get a small note with devtools::check(), but perhaps that is because my R is outdated? It seems fnd should be allowed according to utils docs.

Authors@R field gives persons with no valid roles: ERC Grant Agreement number 336167 - the CHAI Project [fnd]

Code/API

• The function naming convention general seems consistent, except batch_convert() which seems to follow the object_verb() style, but then there is also convert_output() which is the opposite (although that seems to fit better with the rest of the package’s naming convention of verb_object(), such as clean_measures() and identify_lags()).

Documentation

These are mostly style suggestions, feel free to take them or leave them

• I suggest adding links to functions that are referenced. For example in the clean_measures() details, the change point cpt.mean() function is referenced - it would be awesome if it had a hyperlink to view that function.
• Indicate the parameter type in the documentation (for example Character for a path parameter). Functions I noticed without input type specified:
  • convert_output()
  • batch_convert()
  • identify_lags()
• I wonder if the function returns nothing (such as run_shiny_app() and batch_convert() if the roxygen @return should just not be used rather than stating it returns nothing in the documentation? My inclination is that often if nothing is returned, the Value heading just isn’t used, but I could be mistaken.
• For batch_convert() perhaps instead of “converting the settings…to csv” it should specify “writing the settings…to csv” or “saving the settings…to csv” or “exporting the settings…to csv”? For some reason the word “convert” seemed confusing to me.
• Also for batch_convert() I’m not sure I understand what is meant by “See the data in inst/data to see which one applies.” but perhaps this is just because I am not familiar with MicroPEM output?
• Describe the built in shiny app a bit more in run_shiny_app() — maybe stating something about how this is an app for field work?

Other suggestions

• The README doesn’t have a title with the package name (I think this is usually typical?)
• There is a file find_zeros.R that I believe is blank.

[karawoo]

Package Review

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

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

Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

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

Functionality

• Installation: Installation succeeds as documented.
• Functionality: Any functional claims of the software been confirmed.
• Performance: Any performance claims of the software been confirmed.
• 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.
• 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: 2

---

Review Comments

This seems like a very useful package for working with data from RTI MicroPEM devices. After reviewing the package I wish I had data from one of these devices so that I could visualize it with this package! Also the function chai_alarm() makes me smile 🍵 ⏰

All of my comments are pretty minor.

Documentation

• The \dontrun{} section in micropem_class.R is missing the closing curly brace.

Vignettes

• Could you include a link or description of CHAI when it's first referenced in vignette_ammon? There is a link down in the Shiny app section, but I think it would be helpful if it came earlier since this vignette is the main introduction to the package.
• There is a typo in chai_data_cleaning_vignette - in the section "Removal of entire files": "there were to few" should be "there were too few".
• Also a small typo in vignette_ammon: "loosing" -> "losing".
• I think there's a mistake in the image that shows the MicroPEM output file in vignette_ammon. It says the green section goes into a field called "control", but shouldn't it be "settings"?
• Some of the images in vignette_ammon, particularly the Shiny screenshots, are very big and are cut off on my laptop screen. Would it be possible to resize those?

Style

• Internal functions aren't marked with #' @noRd as the rOpenSci style guide suggests, but I don't think that's a big deal.

Tests

• All tests pass on my machine.
• Overall coverage as reported by covr::package_coverage(): 85.41%
• Unlike @LucyMcGowan I don't get any notes when running devtools::check(). I'm on R 3.4.0.

Miscellaneous

• I also noticed the empty find_zeros.R file.

[maelle]

Mant thanks for your reviews @LucyMcGowan @karawoo! 😀 I will make the changes after useR!.

[sckott]

Thanks for your awesome reviews @LucyMcGowan and @karawoo

[maelle]

Thanks again @LucyMcGowan @karawoo! This was really helpful and I'm really glad you reviewed the package! I've now made all changes (except #' @noRd for internal functions, I hope it's ok @sckott ).

Lucy and Kara, are you ok with my adding you as "rev" in DESCRIPTION to acknowledge your contribution?

[sckott]

@LucyMcGowan @karawoo are you happy with the changes made by @maelle ? And let her know if you're okay with the rev bit

[LucyMcGowan]

yep looks 👍 to me! @maelle I am 👌 with the rev bit - that is quite kind of you!

[sckott]

thanks @LucyMcGowan ✅

[karawoo]

Same on both counts!

[maelle]

Added Lucy and Kara as "rev" + added the review badge.

[sckott]

Approved!

To-dos:

• Transfer the repo to rOpenSci, you know the drill :)

• You've already added reviewers in DESCRIPTION as "rev"

• Add the "Peer-reviewed" badge to your README:

[![](https://ropensci.org/badges/126_status.svg)](https://github.com/ropensci/onboarding/issues/126)

• Fix any links in badges for CI and coverage to point to the ropensci URL
• Activate Zenodo watching the repo
• Tag and create a release so as to create a Zenodo version and DOI
• Submit to JOSS using the Zenodo DOI. We will tag it for expedited review.

Welcome aboard once again! Do you want to do a blog post about? Short or long if so?

[maelle]

@sckott thanks! I transferred the repo, updated CI links etc, did the Zenodo stuff and submitted to JOSS.

I'd love to do a blog post about rtimicropem with some general message reg. scientific devices with weird output format and no central documentation. :-)

[sckott]

🚀

cool - i guess blog posts are going through Stef now, will let her know 😸