Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Submission: chlorpromazineR #307

Closed
11 of 25 tasks
eebrown opened this issue May 24, 2019 · 23 comments
Closed
11 of 25 tasks

Submission: chlorpromazineR #307

eebrown opened this issue May 24, 2019 · 23 comments
Assignees

Comments

@eebrown
Copy link
Member

eebrown commented May 24, 2019

Submitting Author: Eric Brown (@eebrown)
Repository: https://github.com/eebrown/chlorpromazineR
Version submitted: 0.1.0
Editor: @karthik
Reviewer 1: @chasemc
Reviewer 2: @fboehm
Archive: TBD
Version accepted: TBD


  • Paste the full DESCRIPTION file inside a code block below:
Package: chlorpromazineR
Title: Convert Antipsychotic Doses to Chlorpromazine Equivalents
Version: 0.1.0
Authors@R: 
    c(person(given = "Eric",
             family = "Brown",
             role = c("aut", "cre"),
             email = "eb@ericebrown.com"),
      person(given = "Parita",
             family = "Shah",
             role = "aut"),
      person(given = "Julia",
             family = "Kim",
             role = "aut"))
Description: As different antipsychotic medications have different potencies,
    the doses of different medications cannot be directly compared. Various
    strategies are used to convert doses into a common reference so that
    comparison is meaningful. Chlorpromazine (CPZ) has historically been used
    as a reference medication into which other antipsychotic doses can be
    converted, as "chlorpromazine-equivalent doses". Using conversion keys
    generated from widely-cited scientific papers (Gardner et. al 2010
    <https://doi.org/10.1176/appi.ajp.2009.09060802>, Leucht et al. 2016
    <https://doi.org/10.1093/schbul/sbv167>), antipsychotic doses are converted
    to CPZ (or any specified antipsychotic) equivalents. The use of the package
    is described in the included vignette. Not for clinical use.
URL: https://github.com/eebrown/chlorpromazineR
BugReports: https://github.com/eebrown/chlorpromazineR/issues
Depends: R (>= 3.5)
License: GPL-3 + file LICENSE
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.1.1
Suggests: 
    knitr,
    rmarkdown,
    testthat,
    covr
VignetteBuilder: knitr

Scope

  • Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

    • data retrieval
    • data extraction
    • database access
    • data munging
    • data deposition
    • reproducibility
    • geospatial data
    • text analysis
  • Explain how and why the package falls under these categories (briefly, 1-2 sentences):

(See #306 - accepted presubmission inquiry) This package falls within "data munging" by virtue of its main feature - to convert antipsychotic doses into a common reference equivalent. It fosters reproducibility as the current practice is typically to reference the source paper without showing the conversion method (which if done manually, though simple, is prone to error). This improves accuracy and transparency.

  • Who is the target audience and what are scientific applications of this package?

Researchers in psychiatry/neuroscience involving antipsychotic medications often need to calculate equivalent doses, which is why the papers describing such methods are widely cited. Yet, there is no R package that facilitates this.

None to our knowledge.

  • If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

#306 @noamross

Technical checks

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

Publication options

JOSS Options
  • The package has an obvious research application according to JOSS's definition.
    • The package contains a paper.md matching JOSS's requirements 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)
MEE Options
  • The package is novel and will be of interest to the broad readership of the journal.
  • The manuscript describing the package is no longer than 3000 words.
  • You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
  • (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
  • (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
  • (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

@karthik
Copy link
Member

karthik commented May 24, 2019

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

👋 @eebrown! I'll be your editor on this submission. A goodpractice report on your packages comes out mostly clean! Once I assign reviewers, we'll take a deeper dive through your package. Here are some small housekeeping things to do in the meantime.

  • Add a badge to your repo. It will update as the review progresses and change to 'reviewed' once accepted.
[![](https://badges.ropensci.org/307_status.svg)](https://github.com/ropensci/onboarding/issues/307)

I'd encourage you to register for an ORCID and add it to your description for each author. See more here

  • There is a small typo in the README.

With this package, he hope:

he should be we?

I'll start looking for reviewers now. If you have suggestions on people who might be qualified to review this (but have no conflict of interest), feel free to suggest names.

@eebrown
Copy link
Member Author

eebrown commented May 24, 2019

Thanks, @karthik! I've updated the DESCRIPTION and README files.

@eebrown
Copy link
Member Author

eebrown commented Jul 15, 2019

Hi @karthik. Any luck finding any reviewers?

@karthik
Copy link
Member

karthik commented Aug 8, 2019

Eric: I'm very embarrassed to have dropped the ball on this amidst late May/June travels. I'm now actively seeking reviewers and will update the thread within a week. I'll make sure to keep this on a fast track the rest of the way. Again, apologies for my tardiness.

@karthik
Copy link
Member

karthik commented Aug 9, 2019

Assigning @chasemc as reviewer 1. Please see guidelines here: https://devguide.ropensci.org/reviewerguide.html

@karthik
Copy link
Member

karthik commented Aug 14, 2019

Still seeking reviewer #2

@karthik
Copy link
Member

karthik commented Aug 15, 2019

Assigning @fboehm as reviewer 2
Please see guidelines here: devguide.ropensci.org/reviewerguide.html

@chasemc
Copy link

chasemc commented Aug 28, 2019

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

Chase's note- My review and inserted links correspond to the chlorpromazineR repository at git commit: f8a0f152fc57ec0afb0c60910a9764dee780508f

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

For packages co-submitting to JOSS
  • NA

Functionality

  • Installation: Installation succeeds as documented.

Session info for installation test:

> sessionInfo()
R version 3.6.1 (2019-07-05)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 17763)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252
[2] LC_CTYPE=English_United States.1252
[3] LC_MONETARY=English_United States.1252
[4] LC_NUMERIC=C
[5] LC_TIME=English_United States.1252

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.2        ps_1.3.0          prettyunits_1.0.2 rprojroot_1.3-2
 [5] digest_0.6.20     crayon_1.3.4      withr_2.1.2       assertthat_0.2.1
 [9] R6_2.4.0          backports_1.1.4   magrittr_1.5      rlang_0.4.0
[13] cli_1.1.0         curl_4.0          fs_1.3.1          remotes_2.1.0
[17] testthat_2.2.1    callr_3.3.1       desc_1.2.0        devtools_2.1.0
[21] tools_3.6.1       glue_1.3.1        pkgload_1.0.2     compiler_3.6.1
[25] processx_3.4.1    pkgbuild_1.0.4    sessioninfo_1.1.1 memoise_1.1.0
[29] usethis_1.5.1
  • Functionality: Any functional claims of the software been confirmed.

    • Vignette and examples were reproduced. Also, the unit tests are sufficient to ensure the authors are recieving expected results (e.g. hard-coded expected results are tested against).
  • 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 my local machine.

==> devtools::test()

Loading chlorpromazineR
Testing chlorpromazineR
v |  OK F W S | Context
v |  19       | checking functions behave [0.1 s]
v |  10       | CPZ-equivalent conversion
v |  15       | checking key manipulation functions

== Results =============================
Duration: 0.3 s

OK:       44
Failed:   0
Warnings: 0
Skipped:  0
  • 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: 5 (partly getting used to/reading rOpenSci procedure)

  • Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

Community guidelines

  • Contribution guidelines are missing.

Packaging guidelines

  • Not sure if this is in the guidelines but I would recommend removing the license section from the README as it is not self-sufficient and may confuse potential users.

  • "Brief demonstration usage" should be added to the README

I would suggest removing "age" from the vignettes/examples, as it suggests it might be used in the conversion. Alternately, change to another "metadata" that obviously wouldn't be used in dosing decisions, or make it clear that age isn't accounted for in the conversion.

Error Messages

Consider making a clear error message for likely things like someone providing a route that isn't accepted:

> to_cpz(example_oral, ap_label = "antipsychotic", dose_label = "dose", 
+        route = "nope")

Error in check_ap(x, key = key, ap_label = ap_label, route = route, route_label = route_label) : 
  object 'notfound' not found

Code Style
There are some places that could benefit from writing code in clearer style, such as using spaces- like here (not required but rOpenSci does recommend http://r-pkgs.had.co.nz/r.html). And, while not incorrect, it would make the code easier trace if that same section was broken into steps and/or code comments were added.

Previous Similar Work
There has been some previous similar work, the Stefan Leucht paper cited in this package (https://dx.doi.org/10.1093%2Fschbul%2Fsbv167) links to an excel tool "Antipsychotic dose conversion website.xls" available from:
http://www.cfdm.de/indexab2e.html?option=com_content&task=view&id=15&Itemid=29
While the Leucht paper is cited as a source of data, the tool is not and probably should be.

Overall
Otherwise, the package was well laid out and had low dependencies and therefore limited worry about future breaking changes.

@fboehm
Copy link

fboehm commented Sep 2, 2019

Overall, I like this package and think that it is useful and well written. I make a few suggestions below that, I hope, may improve the user interface and ease of use. I also include the R code (and output) for the checks that I ran.

Being a new to the package, I tried to think about ways that a new user might guess incorrectly about function use. I encourage the authors to clarify error messages so as to guide the new user towards the correct way to specify the functions' inputs.

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • 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

While the authors state that the package is not for clinical use, I didn't notice an explicit statement of who the target audience is.

  • 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).

I don't know who the maintainer is. Maybe this information can be made explicit in the DESCRIPTION file?

Also, I don't see clear guidelines for contributions.

For packages co-submitting to JOSS

The package contains a paper.md matching JOSS's requirements 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:

  • Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

Reviewer: @fboehm

Review Submitted:




This report contains documents associated with the review of rOpenSci submitted package:

chlorpromazineR: ropensci/software-review issue #307.


Package info

Description:

As different antipsychotic medications have different potencies,
the doses of different medications cannot be directly compared. Various
strategies are used to convert doses into a common reference so that
comparison is meaningful. Chlorpromazine (CPZ) has historically been used
as a reference medication into which other antipsychotic doses can be
converted, as "chlorpromazine-equivalent doses". Using conversion keys
generated from widely-cited scientific papers (Gardner et. al 2010
https://doi.org/10.1176/appi.ajp.2009.09060802, Leucht et al. 2016
https://doi.org/10.1093/schbul/sbv167), antipsychotic doses are converted
to CPZ (or any specified antipsychotic) equivalents. The use of the package
is described in the included vignette. Not for clinical use.

Author: Eric Brown eb@ericebrown.com [aut, cre] (https://orcid.org/0000-0002-1575-2606), Parita Shah [aut] (https://orcid.org/0000-0002-7302-0411), Julia Kim [aut] (https://orcid.org/0000-0002-0379-1333)

repo url: https://github.com/eebrown/chlorpromazineR

website url: <>

Review info

See reviewer guidelines for further information on the rOpenSci review process.

key review checks:

The Mozilla guide divides review questions into two categories: Intrinsic and Extrinsic. I reproduce this structure below.

Intrinsic examination

  1. Are functions as simple as possible?

Yes, the functions are as simple as possible.

  1. Is the code efficient?

Yes.

  1. Is the usage of each function clear?

Yes, the functions have clear usages.

  1. Have edge cases been considered?

I'm not sure what counts as an edge case in this setting. The checks that run on inputs to to_cpz, and possibly other functions, may be insufficient. While I recognize that the to_cpz help file tells the user that argument x is a dateframe, I think it would be helpful for to_cpz code to verify that x is a dateframe. For example, the base function is.data.frame might be used.

Extrinsic examination

  1. Does the new code reinvent any wheels?

No.

  1. Does the new code successfully address the needs of the project?

Yes, I think it does.

  1. Does the new code respect the structure of the project?

Yes.

I'm not sure where to comment on the data. My question is "Would it be useful to provide the data as csv files in addition to rda files?" It looks like you have a directory, "extdata", that contains csv files, yet "extdata" is not part of the submitted package. My thought is that a user may wish to verify the translation and data entry of the tables from the publications into the csv files. (I'm assuming that you entered the results by hand by reading the values from the publications and typing them into a csv.) Providing the csv files may make that easier to verify.

Yes, it complies.

  • Are there improvements that could be made to the code style?

No. In reading the code in the three files within the R/ directory, I feel that the code style is satisfactory.

  • Is there code duplication in the package that should be reduced?

No, I didn't see any code duplication.

  • Are there user interface improvements that could be made?

Would it be sensible to add a column of units to dataframes in your examples in the vignette? You may want to add a column for dosage units and a column for units on the "q" variable. Also, is there a better (more informative) name for the "q" variable? Maybe "frequency" or "period"?

I also suggest adding example code for the vignette section "Participants on multiple medications". I'm unsure what the guidelines suggest for those on multiple anti-psychotics. When computing "chlorpromazine-equivalent" for a participant on multiple drugs, does one merely add the chlorpromazine-equivalent values for each drug?

Also, regarding the names of drugs, is it reasonable to assume that a given drug is only available as a single "salt"? For example, when speaking of "fluphenazine", you use the term "fluphenazine hcl". What if a subject takes fluphenazine in a compound that is not "fluphenazine hcl"? It seems that one would need distinct conversion factors for each compound. For this reason, I am unsure whether I like the use of functions that "trim" the drug names.

  • Are there performance improvements that could be made?

No. The calculations are straightforward. I don't see how performance could be improved.

  • Is the documentation (installation instructions/vignettes/examples/demos) clear and sufficient?

Documentation is both clear and sufficient.

Please be respectful and kind to the authors in your reviews. The rOpenSci code of conduct is mandatory for everyone involved in our review process.


session info

sessionInfo()
#> R version 3.6.1 (2019-07-05)
#> Platform: x86_64-apple-darwin15.6.0 (64-bit)
#> Running under: macOS Mojave 10.14.6
#> 
#> Matrix products: default
#> BLAS:   /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRblas.0.dylib
#> LAPACK: /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRlapack.dylib
#> 
#> locale:
#> [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] devtools_2.1.0 usethis_1.5.1  magrittr_1.5  
#> 
#> loaded via a namespace (and not attached):
#>  [1] Rcpp_1.0.2.1        knitr_1.24          pkgload_1.0.2      
#>  [4] R6_2.4.0            rlang_0.4.0         stringr_1.4.0      
#>  [7] tools_3.6.1         pkgbuild_1.0.5.9000 xfun_0.9           
#> [10] sessioninfo_1.1.1   cli_1.1.0           withr_2.1.2        
#> [13] remotes_2.1.0       htmltools_0.3.6     rprojroot_1.3-2    
#> [16] yaml_2.2.0          digest_0.6.20       assertthat_0.2.1   
#> [19] crayon_1.3.4        processx_3.4.1      callr_3.3.1        
#> [22] fs_1.3.1            ps_1.3.0            testthat_2.2.1     
#> [25] memoise_1.1.0       glue_1.3.1          evaluate_0.14      
#> [28] rmarkdown_1.15      stringi_1.4.3       compiler_3.6.1     
#> [31] backports_1.1.4     desc_1.2.0          prettyunits_1.0.2

Test installation

test local chlorpromazineR install:

install(pkg_dir, dependencies = TRUE, build_vignettes = TRUE)
#> 
#>   
   checking for file/Users/frederickboehm/work-2019/peer-review/ropensci/reviews/chlorpromazineR/DESCRIPTION...checking for file/Users/frederickboehm/work-2019/peer-review/ropensci/reviews/chlorpromazineR/DESCRIPTION#> preparingchlorpromazineR:
#> 
  
   checking DESCRIPTION meta-information ...checking DESCRIPTION meta-information
#> installing the package to build vignettes
#> 
  
   creating vignettes ...creating vignettes (2.5s)
#> checking for LF line-endings in source and make files and shell scripts
#> checking for empty or unneeded directories
#> looking to see if adata/datalistfile should be added
#> buildingchlorpromazineR_0.1.0.tar.gz#> 
  
   
#> 
Running /Library/Frameworks/R.framework/Resources/bin/R CMD INSTALL \
#>   /var/folders/wd/lxmyvz590xb81c5z1j88b3800000gn/T//RtmpboPMAb/chlorpromazineR_0.1.0.tar.gz \
#>   --install-tests 
#> * installing to library ‘/Library/Frameworks/R.framework/Versions/3.6/Resources/library’
#> * installing *source* package ‘chlorpromazineR’ ...
#> ** using staged installation
#> ** R
#> ** data
#> *** moving datasets to lazyload DB
#> ** inst
#> ** tests
#> ** byte-compile and prepare package for lazy loading
#> ** help
#> *** installing help indices
#> ** building package indices
#> ** installing vignettes
#> ** testing if installed package can be loaded from temporary location
#> ** testing if installed package can be loaded from final location
#> ** testing if installed package keeps a record of temporary installation path
#> * DONE (chlorpromazineR)
remove.packages("chlorpromazineR")
#> Removing package from '/Library/Frameworks/R.framework/Versions/3.6/Resources/library'
#> (as 'lib' is unspecified)

comments:

Easily installed with no warnings or errors. No R package dependencies.


test install of chlorpromazineR from GitHub with:

devtools::install_github("eebrown/chlorpromazineR", dependencies = TRUE, build_vignettes = TRUE)
#> Downloading GitHub repo eebrown/chlorpromazineR@master
#> 
#>   
   checking for file/private/var/folders/wd/lxmyvz590xb81c5z1j88b3800000gn/T/RtmpboPMAb/remoteseff855670471/eebrown-chlorpromazineR-f8a0f15/DESCRIPTION...checking for file/private/var/folders/wd/lxmyvz590xb81c5z1j88b3800000gn/T/RtmpboPMAb/remoteseff855670471/eebrown-chlorpromazineR-f8a0f15/DESCRIPTION#> preparingchlorpromazineR:
#> 
  
   checking DESCRIPTION meta-information ...checking DESCRIPTION meta-information
#> installing the package to build vignettes
#> 
  
   creating vignettes ...creating vignettes (2.2s)
#> checking for LF line-endings in source and make files and shell scripts
#> checking for empty or unneeded directories
#> ─  looking to see if a ‘data/datalist’ file should be added
#> ─  building ‘chlorpromazineR_0.1.0.tar.gz’
#> 
  
   
#> 

comments:

Easily installed.


Check package integrity

run checks on chlorpromazineR source:

devtools::check(pkg_dir)
#> Updating chlorpromazineR documentation
#> Writing NAMESPACE
#> Loading chlorpromazineR
#> Writing NAMESPACE
#> ── Building ─────────────────────────────────────────────────────────────────────── chlorpromazineR ──
#> Setting env vars:
#> ● CFLAGS    : -Wall -pedantic
#> ● CXXFLAGS  : -Wall -pedantic
#> ● CXX11FLAGS: -Wall -pedantic
#> ──────────────────────────────────────────────────────────────────────────────────────────────────────
#>   
   checking for file/Users/frederickboehm/work-2019/peer-review/ropensci/reviews/chlorpromazineR/DESCRIPTION...checking for file/Users/frederickboehm/work-2019/peer-review/ropensci/reviews/chlorpromazineR/DESCRIPTION#> preparingchlorpromazineR:
#> 
  
   checking DESCRIPTION meta-information ...checking DESCRIPTION meta-information
#> installing the package to build vignettes
#> 
  
   creating vignettes ...creating vignettes (2.4s)
#> checking for LF line-endings in source and make files and shell scripts
#> checking for empty or unneeded directories
#> ─  looking to see if a ‘data/datalist’ file should be added
#> buildingchlorpromazineR_0.1.0.tar.gz#> 
  
   
#> 
── Checking ─────────────────────────────────────────────────────────────────────── chlorpromazineR ──
#> Setting env vars:
#> ● _R_CHECK_CRAN_INCOMING_REMOTE_: FALSE
#> ● _R_CHECK_CRAN_INCOMING_       : FALSE
#> ● _R_CHECK_FORCE_SUGGESTS_      : FALSE
#> ── R CMD check ────────────────────────────────────────────────────────────
#> * using log directory ‘/private/var/folders/wd/lxmyvz590xb81c5z1j88b3800000gn/T/RtmpboPMAb/chlorpromazineR.Rcheck’
#> * using R version 3.6.1 (2019-07-05)
#> * using platform: x86_64-apple-darwin15.6.0 (64-bit)
#> * using session charset: UTF-8
#> * using options ‘--no-manual --as-cran’
#> * checking for file ‘chlorpromazineR/DESCRIPTION’ ... OK
#> * this is package ‘chlorpromazineR’ version ‘0.1.0’
#> * package encoding: UTF-8
#> * checking package namespace information ... OK
#> * checking package dependencies ... OK
#> * checking if this is a source package ... OK
#> * checking if there is a namespace ... OK
#> * checking for executable files ... OK
#> * checking for hidden files and directories ... OK
#> * checking for portable file names ... OK
#> * checking for sufficient/correct file permissions ... OK
#> * checking whether package ‘chlorpromazineR’ can be installed ... OK
#> * checking installed package size ... OK
#> * checking package directory ... OK
#> * checking for future file timestamps ... OK
#> * checking ‘build’ directory ... OK
#> * checking DESCRIPTION meta-information ... OK
#> * checking top-level files ... OK
#> * checking for left-over files ... OK
#> * checking index information ... OK
#> * checking package subdirectories ... OK
#> * checking R files for non-ASCII characters ... OK
#> * checking R files for syntax errors ... OK
#> * checking whether the package can be loaded ... OK
#> * checking whether the package can be loaded with stated dependencies ... OK
#> * checking whether the package can be unloaded cleanly ... OK
#> * checking whether the namespace can be loaded with stated dependencies ... OK
#> * checking whether the namespace can be unloaded cleanly ... OK
#> * checking dependencies in R code ... OK
#> * checking S3 generic/method consistency ... OK
#> * checking replacement functions ... OK
#> * checking foreign function calls ... OK
#> * checking R code for possible problems ... OK
#> * checking Rd files ... OK
#> * checking Rd metadata ... OK
#> * checking Rd line widths ... OK
#> * checking Rd cross-references ... OK
#> * checking for missing documentation entries ... OK
#> * checking for code/documentation mismatches ... OK
#> * checking Rd \usage sections ... OK
#> * checking Rd contents ... OK
#> * checking for unstated dependencies in examples ... OK
#> * checking contents of ‘data’ directory ... OK
#> * checking data for non-ASCII characters ... OK
#> * checking data for ASCII and uncompressed saves ... OK
#> * checking installed files from ‘inst/doc’ ... OK
#> * checking files in ‘vignettes’ ... OK
#> * checking examples ... OK
#> * checking for unstated dependencies in ‘tests’ ... OK
#> * checking tests ...
#>   Running ‘testthat.R’
#>  OK
#> * checking for unstated dependencies in vignettes ... OK
#> * checking package vignettes in ‘inst/doc’ ... OK
#> * checking re-building of vignette outputs ... OK
#> * DONE
#> 
#> Status: OK
#> ── R CMD check results ───────────────────────── chlorpromazineR 0.1.0 ────
#> Duration: 16.7s
#> 
#> 0 errors ✔ | 0 warnings ✔ | 0 notes ✔

comments:


run tests on chlorpromazineR source:

devtools::test(pkg_dir)
#> Loading chlorpromazineR
#> 
#> Attaching package: 'testthat'
#> The following object is masked from 'package:devtools':
#> 
#>     test_file
#> The following objects are masked from 'package:magrittr':
#> 
#>     equals, is_less_than, not
#> Testing chlorpromazineR
#> ✔ |  OK F W S | Context
#> |   0       | checking functions behave|  19       | checking functions behave
#> |   0       | CPZ-equivalent conversion|  10       | CPZ-equivalent conversion
#> |   0       | checking key manipulation functions|  15       | checking key manipulation functions
#> 
#> ══ Results ═══════════════════════════════════════════════════════════════════════════════════════════
#> Duration: 0.2 s
#> 
#> OK:       44
#> Failed:   0
#> Warnings: 0
#> Skipped:  0

comments:

The tests all pass.


check chlorpromazineR for goodpractice:

goodpractice::gp(pkg_dir)
#> Preparing: covr
#> Preparing: cyclocomp
#>   
   checking for file/private/var/folders/wd/lxmyvz590xb81c5z1j88b3800000gn/T/RtmpboPMAb/remoteseff81a5e5b30/chlorpromazineR/DESCRIPTION...checking for file/private/var/folders/wd/lxmyvz590xb81c5z1j88b3800000gn/T/RtmpboPMAb/remoteseff81a5e5b30/chlorpromazineR/DESCRIPTION#> preparingchlorpromazineR:
#> 
  
   checking DESCRIPTION meta-information ...checking DESCRIPTION meta-information
#> 
  
   checking vignette meta-information ...checking vignette meta-information
#> checking for LF line-endings in source and make files and shell scripts
#> checking for empty or unneeded directories
#> looking to see if adata/datalistfile should be added
#> buildingchlorpromazineR_0.1.0.tar.gz#> 
  
   
#> 
#> Preparing: description
#> Preparing: lintr
#> Preparing: namespace
#> Preparing: rcmdcheck
#> ── GP chlorpromazineR ─────────────────────────────────────────────────────
#> 
#> It is good practice to
#> 
#>   ✖ write unit tests for all functions, and all package code in
#>     general. 90% of code lines are covered by test cases.
#> 
#>     R/convert.R:48:NA
#>     R/convert.R:60:NA
#>     R/convert.R:67:NA
#>     R/convert.R:72:NA
#>     R/convert.R:73:NA
#>     ... and 6 more lines
#> 
#> ───────────────────────────────────────────────────────────────────────────

comments:


Check package metadata files

inspect

spell check

devtools::spell_check(pkg_dir)
#> DESCRIPTION does not contain 'Language' field. Defaulting to 'en-US'.
<script data-pagedtable-source type="application/json"> {"columns":[{"label":[""],"name":["_rn_"],"type":[""],"align":["left"]},{"label":["word"],"name":[1],"type":["chr"],"align":["left"]},{"label":["found"],"name":[2],"type":["list"],"align":["right"]}],"data":[{"1":"ajp","2":"","_rn_":"1"},{"1":"al","2":"","_rn_":"2"},{"1":"antipsychotic","2":"","_rn_":"3"},{"1":"Antipsychotic","2":"","_rn_":"4"},{"1":"antipsychotics","2":"","_rn_":"5"},{"1":"Antipsychotics","2":"","_rn_":"6"},{"1":"ap","2":"","_rn_":"7"},{"1":"appi","2":"","_rn_":"8"},{"1":"aripiprazole","2":"","_rn_":"9"},{"1":"asenapine","2":"","_rn_":"10"},{"1":"Baldessarini","2":"","_rn_":"11"},{"1":"Centorrino","2":"","_rn_":"12"},{"1":"chlorpromazine","2":"","_rn_":"13"},{"1":"Chlorpromazine","2":"","_rn_":"14"},{"1":"cpz","2":"","_rn_":"15"},{"1":"CPZ","2":"","_rn_":"16"},{"1":"csv","2":"","_rn_":"17"},{"1":"Dalkey","2":"","_rn_":"18"},{"1":"Danivas","2":"","_rn_":"19"},{"1":"davis","2":"","_rn_":"20"},{"1":"DDD","2":"","_rn_":"21"},{"1":"decanoate","2":"","_rn_":"22"},{"1":"Delphie","2":"","_rn_":"23"},{"1":"doi","2":"","_rn_":"24"},{"1":"DOI","2":"","_rn_":"25"},{"1":"enanthate","2":"","_rn_":"26"},{"1":"et","2":"","_rn_":"27"},{"1":"extdata","2":"","_rn_":"28"},{"1":"fluphenazine","2":"","_rn_":"29"},{"1":"gardner","2":"","_rn_":"30"},{"1":"haloperidol","2":"","_rn_":"31"},{"1":"Hasson","2":"","_rn_":"32"},{"1":"HCl","2":"","_rn_":"33"},{"1":"Heres","2":"","_rn_":"34"},{"1":"https","2":"","_rn_":"35"},{"1":"injectable","2":"","_rn_":"36"},{"1":"injectables","2":"","_rn_":"37"},{"1":"iteratively","2":"","_rn_":"38"},{"1":"JCP","2":"","_rn_":"39"},{"1":"Keeney","2":"","_rn_":"40"},{"1":"lai","2":"","_rn_":"41"},{"1":"LAI","2":"","_rn_":"42"},{"1":"LAIs","2":"","_rn_":"43"},{"1":"leucht","2":"","_rn_":"44"},{"1":"Leucht","2":"","_rn_":"45"},{"1":"Licence","2":"","_rn_":"46"},{"1":"McKenna","2":"","_rn_":"47"},{"1":"memoranda","2":"","_rn_":"48"},{"1":"olanzapine","2":"","_rn_":"49"},{"1":"parenteral","2":"","_rn_":"50"},{"1":"potencies","2":"","_rn_":"51"},{"1":"quetiapine","2":"","_rn_":"52"},{"1":"sai","2":"","_rn_":"53"},{"1":"SAI","2":"","_rn_":"54"},{"1":"sbv","2":"","_rn_":"55"},{"1":"schbul","2":"","_rn_":"56"},{"1":"Venkatasubramanian","2":"","_rn_":"57"},{"1":"walkthrough","2":"","_rn_":"58"},{"1":"WCCfDS","2":"","_rn_":"59"},{"1":"withsai","2":"","_rn_":"60"},{"1":"www","2":"","_rn_":"61"},{"1":"ziprasidone","2":"","_rn_":"62"}],"options":{"columns":{"min":{},"max":[10]},"rows":{"min":[10],"max":[10]},"pages":{}}} </script>

comments:


Check documentation

online documentation: <>

  • Is the documentation (installation instructions/vignettes/examples/demos) clear and sufficient?

test chlorpromazineR function help files:

help(package = "chlorpromazineR")

comments:


test chlorpromazineR vignettes:

vignette(package = "chlorpromazineR")
#> no vignettes found

comments:


Test functionality:

  • Are there user interface improvements that could be made?
  • Are there performance improvements that could be made?
library("chlorpromazineR")
exports <-ls("package:chlorpromazineR")
exports
#>  [1] "add_key"              "check_ap"             "check_key"           
#>  [4] "check_route"          "convert_by_route"     "davis1974"           
#>  [7] "gardner2010"          "gardner2010_withsai"  "has_long_name"       
#> [10] "leucht2016"           "library.dynam.unload" "system.file"         
#> [13] "to_ap"                "to_cpz"               "trim_key"            
#> [16] "woods2003"
to_cpz("aripiprazole")

comments:

It would be useful to give a more informative message to the user when they enter something like the code above. Perhaps amending the code for to_cpz so that it checks whether inputs have appropriate class & dimension and/or are not NULL would be useful. You could write a line that checks whether x is a dataframe, for example.


Inspect code:

  • Does the package comply with the ROpenSci packaging guide?

    The function argument names are well chosen for the most part. Would it be better to rename x in the function to_cpz? Maybe calling it dataframe would work better.

My other (minor) issue is the use of "q" as a variable name in the vignette.

* good [dependency management](https://ropensci.github.io/dev_guide/building.html#package-dependencies)?

Yes, the package has no R package dependencies.

  • Are there improvements that could be made to the code style?
  • Is there code duplication in the package that should be reduced?
pkgreviewr::pkgreview_print_source("chlorpromazineR")
#> ## add_key
#> function(base, added, trim, verbose = TRUE) {
#>  
#>   if (!check_key(base)) stop("base key did not validate.")
#>   if (!check_key(added)) stop("added key did not validate.")      
#> 
#>   if (trim) {
#>     base <- trim_key(base)
#>     added <- trim_key(added)
#>   } else {
#>     if (has_long_name(base) != has_long_name(added)) {
#>       warning(paste("\nThe 2 keys differ in use of single-word names.",
#>                      "Consider checking compatability and using trim = TRUE.",
#>                      "Otherwise there may be duplicates.\n"))
#>     }
#>   }
#>   
#>   merged <- base
#>   
#>   oral_add <- added$oral[!(names(added$oral) %in% names(base$oral))]
#>   sai_add <- added$sai[!(names(added$sai) %in% names(base$sai))]
#>   lai_add <- added$lai[!(names(added$lai) %in% names(base$lai))]
#> 
#>   merged$oral <- c(base$oral, oral_add)
#>   merged$sai <- c(base$sai, sai_add)
#>   merged$lai <- c(base$lai, lai_add)
#>   
#>   if (!check_key(merged)) stop("Output key did not validate.")   
#> 
#>   if (verbose) {
#>     message(paste0(length(oral_add), " (ORAL) were added to the base key (",
#>                    paste(names(oral_add), collapse=", "), ") \n\n",
#>                    length(sai_add), " (SAI) were added to the base key (",
#>                    paste(names(sai_add), collapse=", "), ") \n\n",
#>                    length(lai_add), " (LAI) were added to the base key (",
#>                    paste(names(lai_add), collapse=", "), ") \n\n" ))
#>   }
#> 
#>   return(merged)
#> }
#> <bytecode: 0x7fb5d15cbb20>
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## check_ap
#> function(x, key=chlorpromazineR::gardner2010, ap_label, route, 
#>                      route_label) {
#>     
#>   if (route %in% c("oral", "sai", "lai")) {
#>       notfound <- !(tolower(x[,ap_label]) %in% names(key[[route]]))
#>       bad <- paste0(x[,ap_label][notfound], " (", route, ")\n")
#>   } else if (route == "mixed") {
#>       notfound_oral <- !(tolower(x[x[,route_label]=="oral",][,ap_label])
#>                          %in% names(key$oral))
#>       notfound_sai <- !(tolower(x[x[,route_label]=="sai",][,ap_label])
#>                         %in% names(key$sai))
#>       notfound_lai <- !(tolower(x[x[,route_label]=="lai",][,ap_label])
#>                         %in% names(key$lai))
#>       notfound <- c(notfound_oral, notfound_sai, notfound_lai)
#>         
#>       if (any(notfound_oral)) {
#>           bad1 <- paste(x[x[,route_label]=="oral",][,ap_label][notfound_oral],
#>                         "(oral)\n")
#>       } else bad1 <- NULL
#> 
#>       if (any(notfound_sai)) {
#>           bad2 <- paste(x[x[,route_label]=="sai",][,ap_label][notfound_sai],
#>                         "(sai)\n")
#>       } else bad2 <- NULL
#> 
#>       if (any(notfound_lai)) {
#>           bad3 <- paste(x[x[,route_label]=="lai",][,ap_label][notfound_lai],
#>                         "(lai)\n")
#>       } else bad3 <- NULL
#> 
#>       bad <- c(bad1, bad2, bad3)
#>     }
#>     
#>   if (sum(notfound) > 0) {
#>       message("The following antipsychotics were not found in the key:")
#>       message(bad)
#>   }
#>     
#>   return(sum(notfound))
#> }
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## check_key
#> function(key) {
#>  
#>   if (!(is.list(key))) stop("Key is not a list.")
#>   if (length(key) != 3) stop("Key is not 3 lists.")
#>   
#>   if (!all(names(key) == c("oral", "sai", "lai"))) {
#>     stop("Key list names should be oral, sai and lai")  
#>   } 
#>   
#>   if (!all(unlist(lapply(key, is.list)))) {
#>     stop("Each element of key should be a list")
#>   }
#>   
#>   if (!is.numeric(unlist(lapply(unlist(key), unlist)))) {
#>     stop("Each key list should have numeric data.")
#>   }
#>   
#>   return(TRUE)
#> }
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## check_route
#> function(x, route_label) {
#>     return(all(x[,route_label] %in% c("oral", "sai", "lai")))
#> }
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## convert_by_route
#> function(x, key, ap_label, dose_label, route_label, 
#>                              factor_label, eq_label) {
#> 
#>   for (r in c("oral", "sai", "lai")) {
#>     k <- key[[r]]
#>     x[x[,route_label]==r,][, factor_label] <-
#>                               as.numeric(k[x[x[,route_label]==r,][,ap_label]])
#>     
#>     x[x[,route_label]==r,][, eq_label] <-
#>             apply(x[x[,route_label]==r,][,c(dose_label, factor_label)], 1, prod)
#> 
#>   }
#>   
#>   return(x)
#> }
#> <bytecode: 0x7fb5d2e9d398>
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## davis1974
#> $oral
#> $oral$chlorpromazine
#> [1] 1
#> 
#> $oral$triflupromazine
#> [1] 3.521127
#> 
#> $oral$thioridazine
#> [1] 1.049318
#> 
#> $oral$prochlorperazine
#> [1] 6.993007
#> 
#> $oral$perphenazine
#> [1] 11.23596
#> 
#> $oral$fluphenazine
#> [1] 83.33333
#> 
#> $oral$trifluoperazine
#> [1] 35.71429
#> 
#> $oral$acetophenazine
#> [1] 4.255319
#> 
#> $oral$carphenazine
#> [1] 4.115226
#> 
#> $oral$butaperazine
#> [1] 11.23596
#> 
#> $oral$mesoridazine
#> [1] 1.808318
#> 
#> $oral$piperacetazine
#> [1] 9.52381
#> 
#> $oral$haloperidol
#> [1] 62.5
#> 
#> $oral$chlorprothixene
#> [1] 2.277904
#> 
#> $oral$thiothixene
#> [1] 19.23077
#> 
#> 
#> $sai
#> $sai$chlorpromazine
#> [1] 3
#> 
#> $sai$triflupromazine
#> [1] 10.56338
#> 
#> $sai$thioridazine
#> [1] 3.147954
#> 
#> $sai$prochlorperazine
#> [1] 20.97902
#> 
#> $sai$perphenazine
#> [1] 33.70787
#> 
#> $sai$fluphenazine
#> [1] 250
#> 
#> $sai$trifluoperazine
#> [1] 107.1429
#> 
#> $sai$acetophenazine
#> [1] 12.76596
#> 
#> $sai$carphenazine
#> [1] 12.34568
#> 
#> $sai$butaperazine
#> [1] 33.70787
#> 
#> $sai$mesoridazine
#> [1] 5.424955
#> 
#> $sai$piperacetazine
#> [1] 28.57143
#> 
#> $sai$haloperidol
#> [1] 187.5
#> 
#> $sai$chlorprothixene
#> [1] 6.833713
#> 
#> $sai$thiothixene
#> [1] 57.69231
#> 
#> 
#> $lai
#> list()
#> 
#> --- 
#>  
#> ## gardner2010
#> $oral
#> $oral$chlorpromazine
#> [1] 1
#> 
#> $oral$amisulpride
#> [1] 0.8571429
#> 
#> $oral$aripiprazole
#> [1] 20
#> 
#> $oral$benperidol
#> [1] 120
#> 
#> $oral$clopenthixol
#> [1] 10
#> 
#> $oral$clorprothixene
#> [1] 1.2
#> 
#> $oral$clotiapine
#> [1] 6
#> 
#> $oral$clozapine
#> [1] 1.5
#> 
#> $oral$droperidol
#> [1] 60
#> 
#> $oral$flupenthixol
#> [1] 60
#> 
#> $oral$fluphenazine
#> [1] 50
#> 
#> $oral$haloperidol
#> [1] 60
#> 
#> $oral$levomepromazine
#> [1] 1.5
#> 
#> $oral$loxapine
#> [1] 10
#> 
#> $oral$mesoridazine
#> [1] 2
#> 
#> $oral$methotrimeprazine
#> [1] 2
#> 
#> $oral$molindone
#> [1] 6
#> 
#> $oral$olanzapine
#> [1] 30
#> 
#> $oral$oxypertine
#> [1] 2.5
#> 
#> $oral$paliperidone
#> [1] 66.66667
#> 
#> $oral$pericyazine
#> [1] 12
#> 
#> $oral$perphenazine
#> [1] 20
#> 
#> $oral$pimozide
#> [1] 75
#> 
#> $oral$prochlorperazine
#> [1] 6.818182
#> 
#> $oral$quetiapine
#> [1] 0.8
#> 
#> $oral$remoxipride
#> [1] 2.830189
#> 
#> $oral$risperidone
#> [1] 100
#> 
#> $oral$sertindole
#> [1] 30
#> 
#> $oral$sulpiride
#> [1] 0.75
#> 
#> $oral$thioridazine
#> [1] 1.2
#> 
#> $oral$thiothixene
#> [1] 20
#> 
#> $oral$trifluoperazine
#> [1] 30
#> 
#> $oral$trifluperidol
#> [1] 300
#> 
#> $oral$triflupromazine
#> [1] 6
#> 
#> $oral$ziprasidone
#> [1] 3.75
#> 
#> $oral$zotepine
#> [1] 2
#> 
#> $oral$zuclopenthixol
#> [1] 12
#> 
#> 
#> $sai
#> list()
#> 
#> $lai
#> $lai$`clopenthixol decanoate`
#> [1] 28
#> 
#> $lai$`flupenthixol decanoate`
#> [1] 210
#> 
#> $lai$`fluphenazine decanoate`
#> [1] 336
#> 
#> $lai$`fluphenazine enanthate`
#> [1] 336
#> 
#> $lai$fluspirilene
#> [1] 700
#> 
#> $lai$`haloperidol decanoate`
#> [1] 112
#> 
#> $lai$`perphenazine enanthate`
#> [1] 84
#> 
#> $lai$`pipotiazine palmitate`
#> [1] 168
#> 
#> $lai$`risperidone microspheres`
#> [1] 168
#> 
#> $lai$`zuclopenthixol decanoate`
#> [1] 42
#> 
#> 
#> --- 
#>  
#> ## gardner2010_withsai
#> $oral
#> $oral$chlorpromazine
#> [1] 1
#> 
#> $oral$amisulpride
#> [1] 0.8571429
#> 
#> $oral$aripiprazole
#> [1] 20
#> 
#> $oral$benperidol
#> [1] 120
#> 
#> $oral$clopenthixol
#> [1] 10
#> 
#> $oral$clorprothixene
#> [1] 1.2
#> 
#> $oral$clotiapine
#> [1] 6
#> 
#> $oral$clozapine
#> [1] 1.5
#> 
#> $oral$droperidol
#> [1] 60
#> 
#> $oral$flupenthixol
#> [1] 60
#> 
#> $oral$fluphenazine
#> [1] 50
#> 
#> $oral$haloperidol
#> [1] 60
#> 
#> $oral$levomepromazine
#> [1] 1.5
#> 
#> $oral$loxapine
#> [1] 10
#> 
#> $oral$mesoridazine
#> [1] 2
#> 
#> $oral$methotrimeprazine
#> [1] 2
#> 
#> $oral$molindone
#> [1] 6
#> 
#> $oral$olanzapine
#> [1] 30
#> 
#> $oral$oxypertine
#> [1] 2.5
#> 
#> $oral$paliperidone
#> [1] 66.66667
#> 
#> $oral$pericyazine
#> [1] 12
#> 
#> $oral$perphenazine
#> [1] 20
#> 
#> $oral$pimozide
#> [1] 75
#> 
#> $oral$prochlorperazine
#> [1] 6.818182
#> 
#> $oral$quetiapine
#> [1] 0.8
#> 
#> $oral$remoxipride
#> [1] 2.830189
#> 
#> $oral$risperidone
#> [1] 100
#> 
#> $oral$sertindole
#> [1] 30
#> 
#> $oral$sulpiride
#> [1] 0.75
#> 
#> $oral$thioridazine
#> [1] 1.2
#> 
#> $oral$thiothixene
#> [1] 20
#> 
#> $oral$trifluoperazine
#> [1] 30
#> 
#> $oral$trifluperidol
#> [1] 300
#> 
#> $oral$triflupromazine
#> [1] 6
#> 
#> $oral$ziprasidone
#> [1] 3.75
#> 
#> $oral$zotepine
#> [1] 2
#> 
#> $oral$zuclopenthixol
#> [1] 12
#> 
#> 
#> $sai
#> $sai$`chlorpromazine hcl`
#> [1] 1
#> 
#> $sai$`clotiapine injectable`
#> [1] 2.5
#> 
#> $sai$`fluphenazine hcl`
#> [1] 20
#> 
#> $sai$`haloperidol lactate`
#> [1] 20
#> 
#> $sai$`loxapine hcl`
#> [1] 4
#> 
#> $sai$`mesoridazine besylate`
#> [1] 1
#> 
#> $sai$`olanzapine tartrate`
#> [1] 10
#> 
#> $sai$`perphenazine usp`
#> [1] 10
#> 
#> $sai$`prochlorperazine mesylate`
#> [1] 4.545455
#> 
#> $sai$`promazine hcl`
#> [1] 1
#> 
#> $sai$`trifluoperazine hcl`
#> [1] 20
#> 
#> $sai$`triflupromazine hcl`
#> [1] 1.666667
#> 
#> $sai$`ziprasidone mesylate`
#> [1] 5
#> 
#> $sai$`zuclopenthixol acetate`
#> [1] 2
#> 
#> 
#> $lai
#> $lai$`clopenthixol decanoate`
#> [1] 28
#> 
#> $lai$`flupenthixol decanoate`
#> [1] 210
#> 
#> $lai$`fluphenazine decanoate`
#> [1] 336
#> 
#> $lai$`fluphenazine enanthate`
#> [1] 336
#> 
#> $lai$fluspirilene
#> [1] 700
#> 
#> $lai$`haloperidol decanoate`
#> [1] 112
#> 
#> $lai$`perphenazine enanthate`
#> [1] 84
#> 
#> $lai$`pipotiazine palmitate`
#> [1] 168
#> 
#> $lai$`risperidone microspheres`
#> [1] 168
#> 
#> $lai$`zuclopenthixol decanoate`
#> [1] 42
#> 
#> 
#> --- 
#>  
#> ## has_long_name
#> function(key) {
#> 
#>   oral <- any(grepl(" ", names(key$oral)))
#>   sai <- any(grepl(" ", names(key$sai)))
#>   lai <- any(grepl(" ", names(key$lai)))
#> 
#>   return(any(c(oral, sai, lai)))
#> 
#> }
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## leucht2016
#> $oral
#> $oral$acepromazine
#> [1] 3.0003
#> 
#> $oral$acetophenazine
#> [1] 5.9988
#> 
#> $oral$amisulpride
#> [1] 0.7500188
#> 
#> $oral$aripiprazole
#> [1] 20
#> 
#> $oral$asenapine
#> [1] 14.9925
#> 
#> $oral$benperidol
#> [1] 200
#> 
#> $oral$bromperidol
#> [1] 30.03003
#> 
#> $oral$butaperazine
#> [1] 30.03003
#> 
#> $oral$chlorpromazine
#> [1] 1
#> 
#> $oral$chlorprothixene
#> [1] 1
#> 
#> $oral$clopenthixol
#> [1] 3.0003
#> 
#> $oral$clotiapine
#> [1] 3.749531
#> 
#> $oral$clozapine
#> [1] 1
#> 
#> $oral$dixyrazine
#> [1] 5.9988
#> 
#> $oral$flupentixol
#> [1] 50
#> 
#> $oral$fluphenazine
#> [1] 30.03003
#> 
#> $oral$haloperidol
#> [1] 37.45318
#> 
#> $oral$levomepromazine
#> [1] 1
#> 
#> $oral$levosulpiride
#> [1] 0.7500188
#> 
#> $oral$loxapine
#> [1] 3.0003
#> 
#> $oral$lurasidone
#> [1] 5
#> 
#> $oral$melperone
#> [1] 1
#> 
#> $oral$mesoridazine
#> [1] 1.499925
#> 
#> $oral$molindone
#> [1] 5.9988
#> 
#> $oral$moperone
#> [1] 14.9925
#> 
#> $oral$olanzapine
#> [1] 30.03003
#> 
#> $oral$oxypertine
#> [1] 2.5
#> 
#> $oral$paliperidone
#> [1] 50
#> 
#> $oral$penfluridol
#> [1] 50
#> 
#> $oral$perazine
#> [1] 3.0003
#> 
#> $oral$periciazine
#> [1] 5.9988
#> 
#> $oral$perphenazine
#> [1] 10
#> 
#> $oral$pimozide
#> [1] 75.18797
#> 
#> $oral$pipamperone
#> [1] 1.499925
#> 
#> $oral$pipotiazine
#> [1] 30.03003
#> 
#> $oral$prochlorperazine
#> [1] 3.0003
#> 
#> $oral$promazine
#> [1] 1
#> 
#> $oral$prothipendyl
#> [1] 1.25
#> 
#> $oral$quetiapine
#> [1] 0.7500188
#> 
#> $oral$remoxipride
#> [1] 1
#> 
#> $oral$risperidone
#> [1] 59.88024
#> 
#> $oral$sertindole
#> [1] 18.76173
#> 
#> $oral$sulpiride
#> [1] 0.3749953
#> 
#> $oral$sultopride
#> [1] 0.25
#> 
#> $oral$thiopropazate
#> [1] 5
#> 
#> $oral$thioproperazine
#> [1] 4
#> 
#> $oral$thioridazine
#> [1] 1
#> 
#> $oral$tiapride
#> [1] 0.7500188
#> 
#> $oral$tiotixene
#> [1] 10
#> 
#> $oral$trifluoperazine
#> [1] 14.9925
#> 
#> $oral$trifluperidol
#> [1] 149.2537
#> 
#> $oral$triflupromazine
#> [1] 3.0003
#> 
#> $oral$ziprasidone
#> [1] 3.749531
#> 
#> $oral$zotepine
#> [1] 1.499925
#> 
#> $oral$zuclopenthixol
#> [1] 10
#> 
#> 
#> $sai
#> $sai$acepromazine
#> [1] 5.9988
#> 
#> $sai$aripiprazole
#> [1] 20
#> 
#> $sai$bromperidol
#> [1] 30.03003
#> 
#> $sai$chlorpromazine
#> [1] 3.0003
#> 
#> $sai$chlorprothixene
#> [1] 5.9988
#> 
#> $sai$clopenthixol
#> [1] 3.0003
#> 
#> $sai$clotiapine
#> [1] 3.749531
#> 
#> $sai$clozapine
#> [1] 1
#> 
#> $sai$dixyrazine
#> [1] 10
#> 
#> $sai$droperidol
#> [1] 120.4819
#> 
#> $sai$haloperidol
#> [1] 37.45318
#> 
#> $sai$levomepromazine
#> [1] 3.0003
#> 
#> $sai$melperone
#> [1] 1
#> 
#> $sai$mesoridazine
#> [1] 1.499925
#> 
#> $sai$moperone
#> [1] 14.9925
#> 
#> $sai$olanzapine
#> [1] 30.03003
#> 
#> $sai$perazine
#> [1] 3.0003
#> 
#> $sai$periciazine
#> [1] 14.9925
#> 
#> $sai$perphenazine
#> [1] 30.03003
#> 
#> $sai$prochlorperazine
#> [1] 5.9988
#> 
#> $sai$promazine
#> [1] 3.0003
#> 
#> $sai$prothipendyl
#> [1] 1.25
#> 
#> $sai$remoxipride
#> [1] 1
#> 
#> $sai$sulpiride
#> [1] 0.3749953
#> 
#> $sai$thioproperazine
#> [1] 14.9925
#> 
#> $sai$tiapride
#> [1] 0.7500188
#> 
#> $sai$trifluoperazine
#> [1] 37.45318
#> 
#> $sai$triflupromazine
#> [1] 3.0003
#> 
#> $sai$ziprasidone
#> [1] 7.501875
#> 
#> $sai$zuclopenthixol
#> [1] 10
#> 
#> 
#> $lai
#> $lai$bromperidol
#> [1] 90.90909
#> 
#> $lai$flupentixol
#> [1] 75.18797
#> 
#> $lai$fluphenazine
#> [1] 303.0303
#> 
#> $lai$fluspirilene
#> [1] 434.7826
#> 
#> $lai$haloperidol
#> [1] 90.90909
#> 
#> $lai$olanzapine
#> [1] 30.03003
#> 
#> $lai$paliperidone
#> [1] 120.4819
#> 
#> $lai$perphenazine
#> [1] 42.91845
#> 
#> $lai$pipotiazine
#> [1] 59.88024
#> 
#> $lai$risperidone
#> [1] 111.1111
#> 
#> $lai$zuclopenthixol
#> [1] 20
#> 
#> 
#> --- 
#>  
#> ## library.dynam.unload
#> function (chname, libpath, verbose = getOption("verbose"), file.ext = .Platform$dynlib.ext) 
#> {
#>     dll_list <- .dynLibs()
#>     if (missing(chname) || nchar(chname, "c") == 0L) 
#>         if (.Platform$OS.type == "windows") 
#>             stop("no DLL was specified")
#>         else stop("no shared object was specified")
#>     libpath <- normalizePath(libpath, "/", TRUE)
#>     chname1 <- paste0(chname, file.ext)
#>     file <- if (nzchar(.Platform$r_arch)) 
#>         file.path(libpath, "libs", .Platform$r_arch, chname1)
#>     else file.path(libpath, "libs", chname1)
#>     pos <- which(vapply(dll_list, function(x) x[["path"]] == 
#>         file, NA))
#>     if (!length(pos)) 
#>         if (.Platform$OS.type == "windows") 
#>             stop(gettextf("DLL %s was not loaded", sQuote(chname1)), 
#>                 domain = NA)
#>         else stop(gettextf("shared object %s was not loaded", 
#>             sQuote(chname1)), domain = NA)
#>     if (!file.exists(file)) 
#>         if (.Platform$OS.type == "windows") 
#>             stop(gettextf("DLL %s not found", sQuote(chname1)), 
#>                 domain = NA)
#>         else stop(gettextf("shared object '%s' not found", sQuote(chname1)), 
#>             domain = NA)
#>     if (verbose) 
#>         message(gettextf("now dyn.unload(\"%s\") ...", file), 
#>             domain = NA)
#>     dyn.unload(file)
#>     .dynLibs(dll_list[-pos])
#>     invisible(dll_list[[pos]])
#> }
#> <bytecode: 0x7fb5d56008f0>
#> <environment: namespace:base>
#> --- 
#>  
#> ## system.file
#> function (..., package = "base", lib.loc = NULL, mustWork = FALSE) 
#> {
#>     if (nargs() == 0L) 
#>         return(file.path(.Library, "base"))
#>     if (length(package) != 1L) 
#>         stop("'package' must be of length 1")
#>     packagePath <- find.package(package, lib.loc, quiet = TRUE)
#>     ans <- if (length(packagePath)) {
#>         FILES <- file.path(packagePath, ...)
#>         present <- file.exists(FILES)
#>         if (any(present)) 
#>             FILES[present]
#>         else ""
#>     }
#>     else ""
#>     if (mustWork && identical(ans, "")) 
#>         stop("no file found")
#>     ans
#> }
#> <bytecode: 0x7fb5ceab4928>
#> <environment: namespace:base>
#> --- 
#>  
#> ## to_ap
#> function(x, convert_to_ap="olanzapine", convert_to_route="oral",
#>                    ap_label, dose_label, route="oral",
#>                    key=chlorpromazineR::gardner2010, cpz_eq_label="cpz_eq",
#>                    ref_eq_label="ap_eq", factor_label="cpz_conv_factor",
#>                    route_label=NULL, q=NULL) {
#>   
#>   if (!(convert_to_ap %in% names(key[[convert_to_route]]))) {
#>       stop("The specified convert_to antipsychotic/route is not in the key")
#>   }
#>   
#>   out <- to_cpz(x=x, ap_label=ap_label, dose_label=dose_label, route=route,
#>                 key=key, eq_label=cpz_eq_label, factor_label=factor_label,
#>                 route_label=route_label, q=q)
#>                 
#>   out[,ref_eq_label] <- out[,cpz_eq_label] /
#>                               as.numeric(key[[convert_to_route]][convert_to_ap])
#>   
#>   return(out)
#> }
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## to_cpz
#> function(x, ap_label, dose_label, route="oral", 
#>                    key=chlorpromazineR::gardner2010, eq_label="cpz_eq", 
#>                    factor_label="cpz_conv_factor", route_label=NULL, q=NULL) {
#>   
#>   check_key(key)
#>   
#>   if (check_ap(x, key=key, ap_label=ap_label, route=route,
#>                route_label=route_label) != 0) {
#>     stop("Data contains antipsychotics not in the key")
#>   }
#>   
#>   cpz_conv_factor <- data.frame(NA)
#>   cpz_eq <- data.frame(NA)
#>   names(cpz_conv_factor) <- factor_label
#>   names(cpz_eq) <- eq_label
#>   x <- cbind(x, cpz_conv_factor, cpz_eq)
#>     
#>   x[,ap_label] <- tolower(x[,ap_label])
#>     
#>   if ((length(route) != 1) | !(route %in% c("oral", "sai", "lai", "mixed"))) {
#>     stop("route must be 1 of \"oral\", \"sai\", \"lai\", or \"mixed\"")
#>   }
#> 
#>   if (route %in% c("oral", "sai", "lai")) {
#>     x[,factor_label] <- as.numeric(key[[route]][x[,ap_label]])
#>     x[,eq_label] <- apply(x[,c(dose_label, factor_label)], 1, prod)
#> 
#>     if (route == "lai" && q != 1) x[,eq_label] <- x[,eq_label] / x[,q]
#>   }
#> 
#>   if (route == "mixed") {
#>         
#>     if (!check_route(x, route_label)) stop("Route column must only include
#>                                             oral, sai or lai")
#>         
#>     if (is.null(q)) stop("A column name for the LAI frequency, q, (days)
#>                           must be specified")
#>     if (is.null(route_label)) stop("A column name for the route, 
#>                                    route_label, must be specified.")
#>     if (!is.numeric(x[, q])) stop("q column must be numeric for LAIs (days)")
#>         
#>     x <- convert_by_route(x=x, key=key, ap_label=ap_label, 
#>                           dose_label=dose_label, route_label=route_label, 
#>                           factor_label=factor_label, eq_label=eq_label)
#> 
#>     if (q != 1) {
#>       x[x$route=="lai",][, eq_label] <- x[x[,route_label]=="lai",][, eq_label] /
#>                                         x[x[,route_label]=="lai",][,q]
#>       }
#>   }
#>     
#>   return(x)
#> }
#> <bytecode: 0x7fb5d3f2ecb8>
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## trim_key
#> function(key) {
#> 
#>   if (!check_key(key)) stop("Input key did not validate.")  
#> 
#>   names(key$oral) <- sub("([A-Za-z]+).*", "\\1", names(key$oral))
#>   names(key$sai) <- sub("([A-Za-z]+).*", "\\1", names(key$sai))
#>   names(key$lai) <- sub("([A-Za-z]+).*", "\\1", names(key$lai))
#>   
#>   if (!check_key(key)) stop("Output key did not validate.")
#> 
#>   return(key) 
#> }
#> <environment: namespace:chlorpromazineR>
#> --- 
#>  
#> ## woods2003
#> $oral
#> $oral$chlopromazine
#> [1] 1
#> 
#> $oral$haloperidol
#> [1] 50
#> 
#> $oral$risperidone
#> [1] 50
#> 
#> $oral$olanzapine
#> [1] 20
#> 
#> $oral$quetiapine
#> [1] 1.333333
#> 
#> $oral$ziprasidone
#> [1] 1.666667
#> 
#> $oral$aripiprazole
#> [1] 13.33333
#> 
#> 
#> $sai
#> list()
#> 
#> $lai
#> list()
#> 
#> --- 
#> 
#> $add_key
#> NULL
#> 
#> $check_ap
#> NULL
#> 
#> $check_key
#> NULL
#> 
#> $check_route
#> NULL
#> 
#> $convert_by_route
#> NULL
#> 
#> $davis1974
#> NULL
#> 
#> $gardner2010
#> NULL
#> 
#> $gardner2010_withsai
#> NULL
#> 
#> $has_long_name
#> NULL
#> 
#> $leucht2016
#> NULL
#> 
#> $library.dynam.unload
#> NULL
#> 
#> $system.file
#> NULL
#> 
#> $to_ap
#> NULL
#> 
#> $to_cpz
#> NULL
#> 
#> $trim_key
#> NULL
#> 
#> $woods2003
#> NULL

* might not be suitable for large packages with many exported functions



comments:

Review test suite:

See guidance on testing for further details.

test coverage

covr::package_coverage(pkg_dir)
#> chlorpromazineR Coverage: 90.76%
#> R/key_functions.R: 88.64%
#> R/convert.R: 92.00%

inspect tests

comments:


@eebrown
Copy link
Member Author

eebrown commented Sep 6, 2019

Dear @chasemc and @fboehm, thank you both very much for your thorough and helpful reviews. I will address your suggestions in an update to the package. Unfortunately the timing has not worked out well and I will be out of office for approx. 2 weeks, but plan to update the package quickly upon my return. Warm regards, Eric

@eebrown
Copy link
Member Author

eebrown commented Sep 25, 2019

Dear @chasemc - thanks again for your helpful and thorough review. I've addressed your comments below:

Review Comments

Community guidelines

  • Contribution guidelines are missing.

Thanks - I've added contributing guidelines and templates with rOpenSci’s use_ro_github() - ropensci/chlorpromazineR@db1fa2b

Packaging guidelines

  • Not sure if this is in the guidelines but I would recommend removing the license section from the README as it is not self-sufficient and may confuse potential users.

Good point. I've removed license text from README.md. ropensci/chlorpromazineR@8d2f9c5

Agreed and added. ropensci/chlorpromazineR@307e23f

I would suggest removing "age" from the vignettes/examples, as it suggests it might be used in the conversion. Alternately, change to another "metadata" that obviously wouldn't be used in dosing decisions, or make it clear that age isn't accounted for in the conversion.

I agree that it could be confusing, so I added a clarifying comment. The reason I included it is to show that the data.frame can include all sorts of extra variables, and age is a typical one. ropensci/chlorpromazineR@8dec24c

Error Messages

Consider making a clear error message for likely things like someone providing a route that isn't accepted:

> to_cpz(example_oral, ap_label = "antipsychotic", dose_label = "dose", 
+        route = "nope")

Error in check_ap(x, key = key, ap_label = ap_label, route = route, route_label = route_label) : 
  object 'notfound' not found

Thanks, great point. I've added a help function check_params() to check that the parameters are the expected format. ropensci/chlorpromazineR@da2ced8 ropensci/chlorpromazineR@64b9b89

Code Style
There are some places that could benefit from writing code in clearer style, such as using spaces- like here (not required but rOpenSci does recommend http://r-pkgs.had.co.nz/r.html). And, while not incorrect, it would make the code easier trace if that same section was broken into steps and/or code comments were added.

I've added spacing per the recommendation. ropensci/chlorpromazineR@b241b9f

Previous Similar Work
There has been some previous similar work, the Stefan Leucht paper cited in this package (https://dx.doi.org/10.1093%2Fschbul%2Fsbv167) links to an excel tool "Antipsychotic dose conversion website.xls" available from:
http://www.cfdm.de/indexab2e.html?option=com_content&task=view&id=15&Itemid=29
While the Leucht paper is cited as a source of data, the tool is not and probably should be.

Good idea, thanks. Citations added to README. ropensci/chlorpromazineR@ab21898

Overall
Otherwise, the package was well laid out and had low dependencies and therefore limited worry about future breaking changes.

Thanks again. Please let me know if you feel I did not fully understand or address any of your feedback and I am happy to revisit.

Responses to @fboehm coming soon...

Best,
@eebrown

@eebrown
Copy link
Member Author

eebrown commented Sep 25, 2019

Dear @fboehm,

Thanks very much for your review. Please find my responses below:

Overall, I like this package and think that it is useful and well written. I make a few suggestions below that, I hope, may improve the user interface and ease of use. I also include the R code (and output) for the checks that I ran.

Thanks 👍

Being a new to the package, I tried to think about ways that a new user might guess incorrectly about function use. I encourage the authors to clarify error messages so as to guide the new user towards the correct way to specify the functions' inputs.

I believe this has been addressed with the new check_params() function. ropensci/chlorpromazineR@da2ced8 ropensci/chlorpromazineR@64b9b89

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • 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

While the authors state that the package is not for clinical use, I didn't notice an explicit statement of who the target audience is.

I've added a statement to the README. The target audience is scientists doing clinical research involving antipsychotic medications. ropensci/chlorpromazineR@9a16784

  • 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).

I don't know who the maintainer is. Maybe this information can be made explicit in the DESCRIPTION file?

This is already in the description file. The [cre] role indicates maintainer, per http://r-pkgs.had.co.nz/description.html.

Also, I don't see clear guidelines for contributions.

Thanks - I have now added these using the rOpenSci function use_ro_github() ropensci/chlorpromazineR@db1fa2b

...

Review info

See reviewer guidelines for further information on the rOpenSci review process.

key review checks:

The Mozilla guide divides review questions into two categories: Intrinsic and Extrinsic. I reproduce this structure below.

Intrinsic examination

  1. Are functions as simple as possible?

Yes, the functions are as simple as possible.

  1. Is the code efficient?

Yes.

  1. Is the usage of each function clear?

Yes, the functions have clear usages.

  1. Have edge cases been considered?

I'm not sure what counts as an edge case in this setting. The checks that run on inputs to to_cpz, and possibly other functions, may be insufficient. While I recognize that the to_cpz help file tells the user that argument x is a dateframe, I think it would be helpful for to_cpz code to verify that x is a dateframe. For example, the base function is.data.frame might be used.

As noted above, this has been addressed with check_params() helper function.

Extrinsic examination

  1. Does the new code reinvent any wheels?

No.

  1. Does the new code successfully address the needs of the project?

Yes, I think it does.

  1. Does the new code respect the structure of the project?

Yes.

I'm not sure where to comment on the data. My question is "Would it be useful to provide the data as csv files in addition to rda files?" It looks like you have a directory, "extdata", that contains csv files, yet "extdata" is not part of the submitted package. My thought is that a user may wish to verify the translation and data entry of the tables from the publications into the csv files. (I'm assuming that you entered the results by hand by reading the values from the publications and typing them into a csv.) Providing the csv files may make that easier to verify.

I agree and encourage users to verify the data entry. I included the external data in inst/extdata as recommended in http://r-pkgs.had.co.nz/data.html. I thought this made the most sense, as it is stored in the repository for anyone to verify.

Yes, it complies.

  • Are there improvements that could be made to the code style?

No. In reading the code in the three files within the R/ directory, I feel that the code style is satisfactory.

  • Is there code duplication in the package that should be reduced?

No, I didn't see any code duplication.

  • Are there user interface improvements that could be made?

Would it be sensible to add a column of units to dataframes in your examples in the vignette? You may want to add a column for dosage units and a column for units on the "q" variable. Also, is there a better (more informative) name for the "q" variable? Maybe "frequency" or "period"?

I chose not to add units, because antipsychotics are universally prescribed in mg. Also, as long as all units are the same, the conversion factors would still work.

I chose the name “q”, because it is actually descriptive, e.g. the LAIs are dosed q 2 weeks, q 3 weeks, etc. It could equally be frequency, but q is nice a short.

I also suggest adding example code for the vignette section "Participants on multiple medications". I'm unsure what the guidelines suggest for those on multiple anti-psychotics. When computing "chlorpromazine-equivalent" for a participant on multiple drugs, does one merely add the chlorpromazine-equivalent values for each drug?

As far as I know there are not guidelines and the user should decide how best to treat this scenario depending on their study/rationale.

Also, regarding the names of drugs, is it reasonable to assume that a given drug is only available as a single "salt"? For example, when speaking of "fluphenazine", you use the term "fluphenazine hcl". What if a subject takes fluphenazine in a compound that is not "fluphenazine hcl"? It seems that one would need distinct conversion factors for each compound. For this reason, I am unsure whether I like the use of functions that "trim" the drug names.

It is almost always the case that there is only one compound, so this function is provided for convenience when the names in the user’s data only use the main name. But as you point out, it is a major issue if it introduces ambiguity between two compounds. The user must verify that this is not the case before using the trim function. I’ve added a message to the trim_function to warn the user. ropensci/chlorpromazineR@170fc76

  • Are there performance improvements that could be made?

No. The calculations are straightforward. I don't see how performance could be improved.

  • Is the documentation (installation instructions/vignettes/examples/demos) clear and sufficient?

Documentation is both clear and sufficient.
...

comments:

It would be useful to give a more informative message to the user when they enter something like the code above. Perhaps amending the code for to_cpz so that it checks whether inputs have appropriate class & dimension and/or are not NULL would be useful. You could write a line that checks whether x is a dataframe, for example.

As noted above, this has been addressed with check_params() helper function.

Inspect code:

  • Does the package comply with the ROpenSci packaging guide?

    The function argument names are well chosen for the most part. Would it be better to rename x in the function to_cpz? Maybe calling it dataframe would work better.

I think in rOpenSci and tidyverse functions it is fairly standard to use x as the first parameters/object of a function as the data object that is being acted on. In this way it is clear what is being changed, e.g. when using the pipe operator.

My other (minor) issue is the use of "q" as a variable name in the vignette.

I chose the name “q”, because it is actually descriptive, e.g. the LAIs are dosed q 2 weeks, q 3 weeks, etc. It could equally be frequency, but q is nice a short.

Thanks again and please also let me know if you feel I did not fully understand or address any of your feedback and I am happy to revisit.

Best,
@eebrown

@fboehm
Copy link

fboehm commented Sep 26, 2019

Thanks, @eebrown. I was mistaken about the creator/maintainer being unlisted. Thanks for the correction. I'm still concerned about the variable naming conventions that you use. Maybe it's just my preference that the variables be informatively named, but I especially dislike the use of "x" as a variable name. As a new user, "x" doesn't tell me what the input needs to be. If you prefer to maintain the use of "q" as a variable, that's fine, although I prefer "period" (the inverse of frequency).

@chasemc
Copy link

chasemc commented Sep 26, 2019

Looks good on my comments @eebrown, I've accepted your corrections and recommend approving.

@fboehm, I tend to agree on your comments about x and q. However, in the case of q I think the target users will understand, as it's a common term when dealing with medications. That being said, and while not incorrect to do, I tend to avoid q because it is a base function: base::q().

@eebrown
Copy link
Member Author

eebrown commented Sep 26, 2019

Thanks @fboehm and @chasemc. I've changed the x argument to input_data. I've changed the q argument to q_label to avoid interference with base::q() and for symmetry with the other arguments that refer to column names in the input data.frame. I've updated the documentation, vignettes and tests accordingly as well.

@fboehm
Copy link

fboehm commented Sep 27, 2019

Thanks, @eebrown. I have updated my review checklist to indicate approval recommendation.

@karthik
Copy link
Member

karthik commented Sep 27, 2019

Congrats @eebrown, your submission has been approved! 🎉 Thank you for submitting and @chasemc and @fboehm for thorough and timely reviews. To-dos:

  • Transfer the repo to the rOpenSci organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.

  • Add the rOpenSci footer to the bottom of your README

[![ropensci_footer](https://ropensci.org/public_images/ropensci_footer.png)](https://ropensci.org)

  • Fix any links in badges for CI and coverage to point to the ropensci URL. (We'll turn on the services on our end as needed)

Welcome aboard! We'd also love a blog post about your package, either a short-form intro to it (https://ropensci.org/technotes/) or long-form post with more narrative about its development. ((https://ropensci.org/blog/). If you are, @stefaniebutland will be in touch about content and timing.

@eebrown
Copy link
Member Author

eebrown commented Oct 1, 2019

  • Transfer the repo to the rOpenSci organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
  • Add the rOpenSci footer to the bottom of your README
  • Fix any links in badges for CI and coverage to point to the ropensci URL. (We'll turn on the services on our end as needed)

Welcome aboard! We'd also love a blog post about your package, either a short-form intro to it (https://ropensci.org/technotes/) or long-form post with more narrative about its development. ((https://ropensci.org/blog/). If you are, @stefaniebutland will be in touch about content and timing.

Thanks!

@stefaniebutland
Copy link
Member

Hi @eebrown. Did you tag me because you wanted to contribute a tech note, or was that a coincidence of copied text :-)

@eebrown
Copy link
Member Author

eebrown commented Oct 3, 2019

Hi @stefaniebutland... it was an accident, but I do plan to prepare a tech note for chlorpromazineR, likely after I finish reviewing another package. 👍

@eebrown
Copy link
Member Author

eebrown commented Oct 9, 2019

@fboehm please let me know your ORCID so that I can add you as a reviewer in the DESCRIPTION file. @chasemc you are welcome to as well if you would like.
Thanks again,
Eric

@fboehm
Copy link

fboehm commented Oct 9, 2019

Yes, of course, @eebrown. It's 0000-0002-1644-5931.

@karthik
Copy link
Member

karthik commented Oct 18, 2019

I'm closing this issue but feel free to continue the conversation around crediting reviewers with their ORCID. 🙏

@karthik karthik closed this as completed Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants