Closed GuangchuangYu closed 2 months ago
Hi @GuangchuangYu
Thanks for submitting your package. We are taking a quick look at it and you will hear back from us soon.
The DESCRIPTION file for this package is:
Package: ggtreeSpace
Title: Visualizing Phylomorphospaces using 'ggtree'
Version: 0.99.0
Authors@R: c(
person("Guangchuang", "Yu",
email = "guangchuangyu@gmail.com",
role = c("aut", "cre", "ths", "cph"),
comment = c(ORCID = "0000-0002-6485-8781")
),
person(given = "Li", family = "Lin",
email = "williamlee2220@gmail.com",
role = "ctb"
)
)
Description: This package is a comprehensive visualization tool specifically designed for
exploring phylomorphospace. It not only simplifies the process of
generating phylomorphospace but also enhances it
with the capability to add graphic layers to the plot with
grammar of graphics to create fully annotated phylomorphospaces.
Imports:
akima,
ape,
dplyr,
GGally,
ggplot2,
grid,
ggtree,
phytools,
rlang,
tibble,
tidyr
Suggests:
knitr,
prettydoc,
rmarkdown
License: MIT + file LICENSE
biocViews: Annotation, Visualization, Phylogenetics, Software
BugReports: https://github.com/YuLab-SMU/ggtreeSpace/issues
URL: https://github.com/YuLab-SMU/ggtreeSpace
RoxygenNote: 7.3.1
Encoding: UTF-8
VignetteBuilder: knitr
Could you please provide an abstract/intro section in your vignette that provides motivation for inclusion in Bioconductor and when appropriate a review and comparison to existing Bioconductor packages with similar functionality or scope.
Your package has been added to git.bioconductor.org to continue the pre-review process. A build report will be posted shortly. Please fix any ERROR and WARNING in the build report before a reviewer is assigned or provide a justification on why you feel the ERROR or WARNING should be granted an exception.
IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. All changes should be pushed to git.bioconductor.org moving forward. It is required to push a version bump to git.bioconductor.org to trigger a new build report.
Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): ggtreeSpace_0.99.0.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
git@git.bioconductor.org:packages/ggtreeSpace
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: 020060b9e96169a3fdb27e828a709883058eb22f
@lshep We have updated the vignette with a more detail introduction.
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): ggtreeSpace_0.99.1.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
git@git.bioconductor.org:packages/ggtreeSpace
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.
Hi @GuangchuangYu
Thanks for submitting ggtreeSpace :tada:! Below is my review of your package. Please reply here if anything is unclear or needs any further explanation.
What next?
Please address the comments as best as you can. When you are ready for me to check the package again please reply to let me know with a summary of changes you have made or any other responses. You can use the "Quote reply" option in the ...
menu on this comment to reply directly to my points below.
Luke
Key: :rotating_light: Required :warning: Recommended :green_circle: Optional :question: Question
BiocCheck
notes in the build report, please check these and address them if necessarycamelCase
or snake_case
and do not include .
README
. Any installation instructions should include Bioconductor instructions.BiocStyle
package for formatting vignette. It should include a table of contents.message()
, warning()
, stop()
instead of cat()
...
then it should be clear in the description how additional arguments are used.aes_string()
function from the {ggplot2} function is deprecated and should not be used. Please see the "Using ggplot2 in packages" and "Extending ggplot2" guides for alternatives.Received a valid push on git.bioconductor.org; starting a build for commit id: 9349d714fa377193c73b67959ec6e9c631adaa6b
Thank you for your detailed examination and constructive feedback on our package. We greatly appreciate the time and effort you have invested in reviewing our work. Below, we address your comments as follows:
General package development
Description
section in the DESCRIPTION file and shortening the code lines. However, some notes, such as those regarding function length, require significant code restructuring. We plan to tackle these in forthcoming updates. Many thanks for your valuable suggestions.
DESCRIPTION fileinterp
function from {akima} package with interp
function from {interp} package. NAMESPACE file
camelCase
or snake_case
and do not include .
ggtreeSpace
, geom_treeSpace
, theme_treeSpace1
and theme_treeSpace2
fuction into ggtreespace
, geom_treespace
,theme_treespace1
and theme_treespace2
in order to match the snake_case
. Thank you for pointing out our mistake.Documentation
spell_check_package
function of {spelling} package. Sorry for the inconvenience. README
🚨 Please add more content to the README. Any installation instructions should include Bioconductor instructions.
🚨 Please use the BiocStyle package for formatting vignette. It should include a table of contents.
🚨 Please add an Installation section that:
⚠️ I feel there could be more detail about the functionality of the package including how to interpret the plots and what a "phylomorphospace" is.
ggtreespace
function, and add annotation to it with the +
operator. In this example, we add symbolic point to the tip of phylomorphospace. From the phylomorphospace plot, we can observe the evolutionary trajectories of different species, illustrating how they diverge and adapt in their respective trait dimensions. This visual representation allows us to identify patterns of convergence and divergence among species, highlight instances of adaptive radiation, and so on. "🚨 You only need to load libraries once at the start of the document, not in every chunk
Unit tests
testthat
.
CodeR
🚨 Please use message(), warning(), stop() instead of cat()
phylospm
function. ⚠️ Consider add more detail to function argument descripions. It is not clear to me what all the arguments should be.
#' @title Plot phylomorphospace
#'
#' @description This function plots a phylomorphospace by mapping a tree
#' object onto a vector space like morphospace.
#' @param tr a tree object. This should be an object of class that is
#' compatible with `ggtree`, typically an object of class
#' `phylo` or `treedata`.
#' @param data Trait data as a data frame or matrix, where each row
#' represents a tree tip or node.
#'
#' For data matching the number of tips, ancestral traits are reconstructed
#' for internal nodes.
#'
#' For data equal to the total number of nodes, values are directly used as
#' node coordinates.
#' @param mapping aesthetic mapping
#' @param ... additional parameters for customization with `ggtree`
⚠️ If function arguments include ... then it should be clear in the description how additional arguments are used.
#' @param ... additional parameters for customization with `ggtree`. Please
#' use `?ggtree::ggtree` fot more information.
⚠️ Consider adding validity checks to function arguments
🟢 I think you have included global variables to avoid check warnings but it may be possible to avoid these in other ways
🚨 The aes_string() function from the {ggplot2} function is deprecated and should not be used. Please see the "Using ggplot2 in packages" and "Extending ggplot2" guides for alternatives.
❓ The functions in this package seem to be minimal wrappers around {ggtree} and other packages. Can you please explain what you think the main added value of this package is?
Received a valid push on git.bioconductor.org; starting a build for commit id: 5119cead63170c5840fb7cdbc818285712a80252
⚠️ Ther are some BiocCheck notes in the build report, please check these and address them if necessary
- ✅Response: We have addressed some of the notes by enriching the
Description
section in the DESCRIPTION file and shortening the code lines. However, some notes, such as those regarding function length, require significant code restructuring. We plan to tackle these in forthcoming updates. Many thanks for your valuable suggestions.
There seems to be an issue with the build reports at the moment so I can't check this but sounds like you have addressed this.
🚨 Please add an Installation section that:
- Uses Bioconductor installation instructions
- Installation code is not evaluated
- ✅Response: We have added an Installation section in the vignette. Thanks for the tip.
I think this usually goes after the introduction section but it probably doesn't matter too much. It would be nice to have a bit of text to go with the code though.
🚨 You only need to load libraries once at the start of the document, not in every chunk
- ✅Response: We have fixed this by loading libraries only at the start. Grateful for your keen observations.
Please do this in a separate chunk with include = TRUE
so it is visible to the reader.
Additional point: Please don't disable warnings in the vignette so that the output matches what users will see.
⚠️ Consider add more detail to function argument descripions. It is not clear to me what all the arguments should be.
- ✅Response: We have updated the argument descripions of some main functions, for example:
Please do this for all exported function, some arguments are still missing descriptions
⚠️ If function arguments include ... then it should be clear in the description how additional arguments are used.
- ✅Response: We have enriched the description of function arguments that include ... , for example:
#' @param ... additional parameters for customization with `ggtree`. Please #' use `?ggtree::ggtree` fot more information.
This is also missing in some places (e.g. ggTreeSpace()
)
❓ The functions in this package seem to be minimal wrappers around {ggtree} and other packages. Can you please explain what you think the main added value of this package is?
- ✅Response: While this package indeed serves as a wrapper around the {ggtree} and other related packages, its primary value lies in significantly simplifying the process for users unfamiliar with the underlying structure of {ggtree}. This simplification enables users to efficiently generate phylomorphospace visualizations without delving into the complexities of {ggtree}. Additionally, {ggtreeSpace} serves as a foundational package. In forthcoming updates, we plan to introduce more graphical layers specifically designed for phylomorphospace annotation, thereby enriching the package's capabilities for the community.
Ok. It would be good to cover this in the vignette, maybe by showing an example comparing doing something manually with {ggtree} and your package.
Additional comments:
phylospm()
function but I'm not sure what purpose this has as it doesn't seem to be reused anywhere and there are no functions/methods for this class.Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): ggtreeSpace_0.99.3.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
git@git.bioconductor.org:packages/ggtreeSpace
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Thank you very much for your thorough review. Below is our response to the feedback provided.
⚠️ Ther are some BiocCheck notes in the build report, please check these and address them if necessary
- ✅Response: We have addressed some of the notes by enriching the
Description
section in the DESCRIPTION file and shortening the code lines. However, some notes, such as those regarding function length, require significant code restructuring. We plan to tackle these in forthcoming updates. Many thanks for your valuable suggestions.
There seems to be an issue with the build reports at the moment so I can't check this but sounds like you have addressed this.
🚨 Please add an Installation section that:
- Uses Bioconductor installation instructions
- Installation code is not evaluated
- ✅Response: We have added an Installation section in the vignette. Thanks for the tip.
I think this usually goes after the introduction section but it probably doesn't matter too much. It would be nice to have a bit of text to go with the code though.
🚨 You only need to load libraries once at the start of the document, not in every chunk
- ✅Response: We have fixed this by loading libraries only at the start. Grateful for your keen observations.
Please do this in a separate chunk with include = TRUE
so it is visible to the reader.
Additional point: Please don't disable warnings in the vignette so that the output matches what users will see.
⚠️ Consider add more detail to function argument descripions. It is not clear to me what all the arguments should be.
- ✅Response: We have updated the argument descripions of some main functions, for example:
Please do this for all exported function, some arguments are still missing descriptions
⚠️ If function arguments include ... then it should be clear in the description how additional arguments are used.
- ✅Response: We have enriched the description of function arguments that include ... , for example:
#' @param ... additional parameters for customization with `ggtree`. Please #' use `?ggtree::ggtree` fot more information.
This is also missing in some places (e.g. ggTreeSpace()
)
...
parameters undocumented. Could you please specify where you found them missing? Thank you for your patience.
❓ The functions in this package seem to be minimal wrappers around {ggtree} and other packages. Can you please explain what you think the main added value of this package is?
- ✅Response: While this package indeed serves as a wrapper around the {ggtree} and other related packages, its primary value lies in significantly simplifying the process for users unfamiliar with the underlying structure of {ggtree}. This simplification enables users to efficiently generate phylomorphospace visualizations without delving into the complexities of {ggtree}. Additionally, {ggtreeSpace} serves as a foundational package. In forthcoming updates, we plan to introduce more graphical layers specifically designed for phylomorphospace annotation, thereby enriching the package's capabilities for the community.
Ok. It would be good to cover this in the vignette, maybe by showing an example comparing doing something manually with {ggtree} and your package.
Additional comments:
You assign a class to the output of the phylospm()
function but I'm not sure what purpose this has as it doesn't seem to be reused anywhere and there are no functions/methods for this class.
✅Response: We have removed the class assignment.
Received a valid push on git.bioconductor.org; starting a build for commit id: 986cd7a758c6db951d5d3680a00fea9652b3b94e
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): ggtreeSpace_0.99.4.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
git@git.bioconductor.org:packages/ggtreeSpace
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Thanks for your replies, I have one more comment. The %<+%
operator is not commonly used (I'm not even sure what it does) so I would avoid using it in the vignette.
This is only minor though, and we are close to the deadline so I will accept this package as is and trust you to make this change.
Your package has been accepted. It will be added to the Bioconductor nightly builds.
Thank you for contributing to Bioconductor!
Reviewers for Bioconductor packages are volunteers from the Bioconductor community. If you are interested in becoming a Bioconductor package reviewer, please see Reviewers Expectations.
Congratulations on getting {ggtreeSpace} accepted into Bioconductor 🎉! It can take a few days to be picked up by the build system but then it should be available in Bioconductor devel and the next release.
This operator was defined in {ggtree} package for more than 8 years and it is widely used in {ggtree} and derived packages. I think there is no need to change it. We will add references to it.
The default branch of your GitHub repository has been added to Bioconductor's git repository as branch devel.
To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/GuangchuangYu.keys is not empty), then no further steps are required. Otherwise, do the following:
See further instructions at
https://bioconductor.org/developers/how-to/git/
for working with this repository. See especially
https://bioconductor.org/developers/how-to/git/new-package-workflow/ https://bioconductor.org/developers/how-to/git/sync-existing-repositories/
to keep your GitHub and Bioconductor repositories in sync.
Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at
https://bioconductor.org/checkResults/
(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using BiocManager::install("ggtreeSpace")
. The package 'landing page' will be created at
https://bioconductor.org/packages/ggtreeSpace
If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.
Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor
Confirm the following by editing each check box to '[x]'
[x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.
[x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.
[x] I understand Bioconductor Package Naming Policy and acknowledge Bioconductor may retain use of package name.
[x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.
[x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.
[x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.
[x] I am familiar with the Bioconductor code of conduct and agree to abide by it.
I am familiar with the essential aspects of Bioconductor software management, including:
For questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.