-
-
Notifications
You must be signed in to change notification settings - Fork 104
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: UCSCXenaTools #315
Comments
Editor checks:
Editor commentsThank you for submitting your package! Please add a LICENCE file to the repo, and fix the following suggestions of
Reviewers: @ChristineStawitz-NOAA and @carlganz |
Dear editor @melvidoni, All the comments you mentioned above have been fixed. A GPL-3 license file has been added by Thanks for your comments. Please let me know if there are other issues. Best, |
Thanks @ShixiangWang! I'm looking for reviewers, which may take a little. I will comment again when I assign them, and let you know about the due date. |
Reviewers @ChristineStawitz-NOAA and @carlganz have been assigned to the package. They'll have until July 22nd to get the reviews in. |
Thanks, @melvidoni. Welcome the reviewers👏 👏 👏 |
Apologies for getting this in at the last possible moment! Package Review
DocumentationThe package includes all the following forms of documentation:
Functionality
I get a warning when running
Review CommentsI'm not in the omics field, but this seems like a useful package for interacting with the UCSC Xena platform. I found the package reliable and stable, but it was a bit hard to follow where the vignettes were located after local install and there were a few vignette issues (see above). There are a couple of code simplifications and consistency improvements that could be made as well, which are detailed above. |
Thank you @ChristineStawitz-NOAA! We are waiting for @carlganz's now, then. |
Promise to finish review tomorrow sorry for delay.
… On Jul 22, 2019, at 4:35 PM, Melina Vidoni ***@***.***> wrote:
Thank you @ChristineStawitz-NOAA! We are waiting for @carlganz's now, then.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@ChristineStawitz-NOAA Thanks for your all comments. Basically, I fixed all the problems you raised. And I also have some reasons for the non-modified parts. All replies for problematic points have been put as the following. Please forgive me my English if you find something hard to understand, just ask me again. :)
I get a warning when running devtools::check() - "qpdf is needed for size reduction on PDFs"
|
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:
Functionality
Final approval (post-review)
Estimated hours spent reviewing: 4
Review CommentsI have very little knowledge of genomics, but I can see clearly that the UCSC Xena platform is valuable to the genomics community, and after thoroughly reviewing the code within your package I can see that working with the Xena platform is highly non-trivial, and that this package encapsulates a great deal of work that succesfully makes the Xena platform much more accessible to R users. Congrats on the wonderful package! Statement of Need and JOSS summaryYou explain what the Xena platform is, but it would be nice to have a sentence or two explaining why interfacing with the Xena platform is non-trivial, and how this interface significantly improves R user access. Installation
Small code suggestions
So I would replace this: resDF <- data.frame(stringsAsFactors = FALSE)
for (i in 1:length(XenaList)) {
hostNames <- names(XenaList)[i]
cohortNames <- names(XenaList[[i]])
res <- data.frame(stringsAsFactors = FALSE)
for (j in 1:length(cohortNames)) {
oneCohort <- XenaList[[i]][j]
# The unassigned cohorts have NULL data, remove it
if (names(oneCohort) != "(unassigned)") {
resCohort <- data.frame(
XenaCohorts = names(oneCohort),
XenaDatasets = as.character(oneCohort[[1]]),
stringsAsFactors = FALSE
)
res <- rbind(res, resCohort)
}
}
res$XenaHosts <- hostNames
resDF <- rbind(resDF, res)
} With something like: # use lapply to create list of dfs because why introduce purrr dependency
resDF <- lapply(seq_len(XenaList), function(i) {
hostNames <- names(XenaList)[i]
cohortNames <- names(XenaList[[i]])
res <- data.frame(stringsAsFactors = FALSE)
for (j in 1:length(cohortNames)) {
oneCohort <- XenaList[[i]][j]
# The unassigned cohorts have NULL data, remove it
if (names(oneCohort) != "(unassigned)") {
resCohort <- data.frame(
XenaCohorts = names(oneCohort),
XenaDatasets = as.character(oneCohort[[1]]),
stringsAsFactors = FALSE
)
res <- rbind(res, resCohort)
}
}
res$XenaHosts <- hostNames
return(res)
}) %>%
# convert list of dfs into single df
bind_rows
Hope that helps! |
@ChristineStawitz-NOAA Sorry for ignoring @carlganz Thanks for your comments :). The modification can be seen from the last two commits. I also reply your comments point to point as the following. Statement of Need and JOSS summaryIt was a template in repository yesterday. I rewrite it today, you can see new version here. I am not good at English and also writing, so any suggestion is welcome :). Installationprettydoc package is in suggests field, so it will not be install automatically without option use seq_lenI like use forI won’t change such code for now. I do like For example,
The code won’t work before modifying all When I developed this package, I have no idea how to use As Hadley wrote in Advance in R, the for loop in R now is very quick. It will not affect the performance. use bind_rowsThis is modified. |
I am glad to add two reviewers in DESCRIPTION file if you agree according to
Thanks all your comments @ChristineStawitz-NOAA @carlganz |
Great to see this is moving! Please @ChristineStawitz-NOAA and @carlganz, review the changes. Feel free to go back and forth if you deem it necessary. |
Statement of Need and JOSS paper I initially did not review this as a JOSS submission, should I be also following their review criteria @melvidoni or will it go through a separate review at JOSS after the rOpenSci one? I have some long-running code going in other |
I am happy with the updates. I've updated review to give full approval. Congrats! |
@ChristineStawitz-NOAA the bibliography is shown in paper.bib file according to JOSS instruction. @ChristineStawitz-NOAA @carlganz I have added your names here and here, please take a look and make sure they are right. |
Approved! Thanks @ShixiangWang for submitting and @ChristineStawitz-NOAA @carlganz for your reviews! To-dos:
Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them Welcome aboard! We'd love to host a blog post about your package - either a short introduction to it with one example or a longer post with some narrative about its development or something you learned, and an example of its use. If you are interested, review the instructions, and tag @stefaniebutland in your reply. She will get in touch about timing and can answer any questions. We've started putting together a gitbook with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here. |
@melvidoni I got a message when I transferred the repository
|
@ShixiangWang I sent you an invitation to be part of rOpenSci organisation in GItHub. If you accept it, you should be able to transfer the repo. (Transfer, do not create!). |
@melvidoni Thanks, it works. It seems I should have rights to access the repositories of ropensci. I don't know where the problem comes from, the Zenodo (I just opened an issue) or the setting from ropensci? Please help me . I also found I am in a team |
I removed you from that team @ShixiangWang, and added you to the package team. Can you accept that? (it shows as pending). Also, can you try adding Zenodo now? |
Strange, I was already in the package team yesterday, so why is it showing pending?. I will try Zenodo as soon as possible. |
@melvidoni Thanks, I can find the package in Zenodo now. :) |
Awesome! Then, if you can tackle everything and then let me know, that would be great. |
@melvidoni I have done everything in your todo list. I have submitted URL https://doi.org/10.5281/zenodo.3358530 to JOSS. If the DOI not found, please see https://zenodo.org/record/3358530 @stefaniebutland I would like to write a short introduction to this package. Thanks all. |
Good to hear that @ShixiangWang @melvidoni is there an element of the package, it's development, or applications that you think could be highlighted? For a short introduction, a Tech Note is most appropriate. Examples. It's important not to repeat information that's in the vignette; refer to it where needed. You might illustrate a compelling use case for the package. Editorial and technical information about preparing your submission, including a template: https://github.com/ropensci/roweb2#contributing-a-blog-post. In the template YAML, for a Tech Note, set Please suggest a deadline for submitting your draft. Don't hesitate to ask me questions. I'll be away Mon Aug 5. Thank you for doing this extra work. I hope the post will get more eyes on your work. |
@stefaniebutland Thanks, can I submit the draft within a month? @melvidoni It seems that I should submit repository URL instead of Zenodo DOI to JOSS, please see openjournals/joss-reviews#1622 (comment) |
@ShixiangWang That is weird! If they need URL repo, then that's it. Send the repo. I will advise editors to fix this on the approval comment template as well! |
@melvidoni Yes 😭. Is there anything more I should do? |
Not for now! Just send them the URL repo then. |
@melvidoni The package has published on JOSS https://joss.theoj.org/papers/10.21105/joss.01627. Thanks for your work, the process is really fast. |
@ShixiangWang That is great! If you don't mind, I'm tweeting your package, and closing the thread. Congrats and thank you for all the hard work! |
Yes, sounds good. I'll mark my calendar to check with you on Aug 27. Thank you. |
Submitting Author: ShixiangWang (@ShixiangWang)
Repository: https://github.com/ShixiangWang/UCSCXenaTools
Version submitted: 1.2.3
Editor: @melvidoni
Reviewer 1: @ChristineStawitz-NOAA
Reviewer 2: @carlganz
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):
The package falls under the data retrieval category because it pulls data from UCSC Xena Hubs.
Who is the target audience and what are scientific applications of this package?
Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?
None that I am aware of. There is a Python package called xenaPython which is the official API for Xena Hubs. However, UCSCXenaTools is more powerful and provides more features including download whole dataset and query a small amount of data.
#314
Technical checks
Confirm each of the following by checking the box. This package:
Publication options
This package is already available at CRAN
JOSS Options
paper.md
matching 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: