Submission: chlorpromazineR #307
Comments
Editor checks:
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.
I'd encourage you to register for an ORCID and add it to your description for each author. See more here
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. |
|
Thanks, @karthik! I've updated the DESCRIPTION and README files. |
|
Hi @karthik. Any luck finding any reviewers? |
|
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. |
|
Assigning @chasemc as reviewer 1. Please see guidelines here: https://devguide.ropensci.org/reviewerguide.html |
|
Still seeking reviewer #2 |
|
Assigning @fboehm as reviewer 2 |
Package ReviewPlease 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:
DocumentationThe package includes all the following forms of documentation:
For packages co-submitting to JOSS
Functionality
Session info for installation test:
Final approval (post-review)
Estimated hours spent reviewing: 5 (partly getting used to/reading rOpenSci procedure)
Review CommentsCommunity guidelines
Packaging guidelines
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: Code Style Previous Similar Work Overall |
|
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 ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
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 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.
Functionality
Final approval (post-review)
Estimated hours spent reviewing:
Review CommentsReviewer: @fboehmReview Submitted:This report contains documents associated with the review of rOpenSci submitted package:
|
|
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 |
|
Dear @chasemc - thanks again for your helpful and thorough review. I've addressed your comments below:
Thanks - I've added contributing guidelines and templates with rOpenSci’s use_ro_github() - ropensci/chlorpromazineR@db1fa2b
Good point. I've removed license text from README.md. ropensci/chlorpromazineR@8d2f9c5
Agreed and added. ropensci/chlorpromazineR@307e23f
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
Thanks, great point. I've added a help function
I've added spacing per the recommendation. ropensci/chlorpromazineR@b241b9f
Good idea, thanks. Citations added to README. ropensci/chlorpromazineR@ab21898
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, |
|
Dear @fboehm, Thanks very much for your review. Please find my responses below:
Thanks 👍
I believe this has been addressed with the new
I've added a statement to the README. The target audience is scientists doing clinical research involving antipsychotic medications. ropensci/chlorpromazineR@9a16784
This is already in the description file. The [cre] role indicates maintainer, per http://r-pkgs.had.co.nz/description.html.
Thanks - I have now added these using the rOpenSci function use_ro_github() ropensci/chlorpromazineR@db1fa2b
As noted above, this has been addressed with
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.
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.
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.
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
As noted above, this has been addressed with
I think in rOpenSci and tidyverse functions it is fairly standard to use
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, |
|
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). |
|
Looks good on my comments @eebrown, I've accepted your corrections and recommend approving. @fboehm, I tend to agree on your comments about |
|
Thanks @fboehm and @chasemc. I've changed the |
|
Thanks, @eebrown. I have updated my review checklist to indicate approval recommendation. |
|
Congrats @eebrown, your submission has been approved! 🎉 Thank you for submitting and @chasemc and @fboehm for thorough and timely reviews. To-dos:
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! |
|
Hi @eebrown. Did you tag me because you wanted to contribute a tech note, or was that a coincidence of copied text :-) |
|
Hi @stefaniebutland... it was an accident, but I do plan to prepare a tech note for chlorpromazineR, likely after I finish reviewing another package. 👍 |
|
Yes, of course, @eebrown. It's 0000-0002-1644-5931. |
|
I'm closing this issue but feel free to continue the conversation around crediting reviewers with their ORCID. 🙏 |
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
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.):
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.
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.
#306 @noamross
Technical checks
Confirm each of the following by checking the box. This package:
Publication options
JOSS Options
paper.mdmatching JOSS's requirements with a high-level description in the package root or ininst/.MEE Options
Code of conduct
The text was updated successfully, but these errors were encountered: