Closed trvinh closed 5 years ago
Hi @trvinh
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: PhyloProfile
Version: 1.0.0
Date: 2018-12-12
Title: PhyloProfile
Authors@R: c(
person("Vinh", "Tran", email = "tran@bio.uni-frankfurt.de", role = c("aut", "cre")),
person("Bastian", "Greshake Tzovaras", email = "", role = "aut"),
person("Ingo", "Ebersberger", email = "ebersberger@bio.uni-frankfurt.de", role = "aut"),
person("Carla", "Mölbert", email = "carla.moelbert@gmx.de", role = "ctb"))
Maintainer: Vinh Tran <tran@bio.uni-frankfurt.de>
Description: Analyze multidimensional phylogenetic profiles with a Shiny app.
URL: https://github.com/BIONF/PhyloProfile/
BugReports: https://github.com/BIONF/PhyloProfile/issues
License: MIT + file LICENSE
Depends: R (>= 3.5.0)
Encoding: UTF-8
biocViews: Software, Visualization, DataRepresentation, MultipleComparison, FunctionalPrediction
Imports:
ape (>= 5.0),
bioDist (>= 1.52.0),
BiocStyle (>= 2.0.0),
Biostrings (>= 2.48.0),
Cairo (>= 1.5.9),
colourpicker (>= 1.0),
data.table (>= 1.10.4),
dendextend (>= 1.8.0),
devtools (>= 1.13.6),
dplyr (>= 0.7.4),
DT (>= 0.4),
energy (>= 1.7),
GenomeInfoDbData (>= 1.1.0),
ggplot2 (>= 2.2.1),
gplots (>= 3.0.1),
GO.db (>= 3.6.0),
grDevices (>= 3.5.0),
gridExtra (>= 2.3),
gtable (>= 0.2.0),
knitr (>= 1.21),
RColorBrewer (>= 1.1),
RCurl (>= 1.95),
reshape2 (>= 1.4.3),
rmarkdown (>= 1.11),
scales (>= 0.5.0),
shiny (>= 1.1.0),
shinyBS (>= 0.61),
shinycssloaders (>= 0.2.0),
shinyjs (>= 1.0),
stats (>= 3.5.0),
stringr (>= 1.2.0),
svMisc (>= 1.1.0),
tidyr (>= 0.8.1),
taxize (>= 0.9.0),
utils (>= 3.5.0),
OmaDB (>= 1.1.1),
plyr (>= 1.8.4),
zoo (>= 1.8)
RoxygenNote: 6.1.1
Remotes:
trvinh/OmaDB,
andrewsali/shinycssloaders
Suggests:
testthat
VignetteBuilder: knitr
Add SSH keys to your GitHub account. SSH keys will are used to control access to accepted Bioconductor packages. See these instructions to add SSH keys to your GitHub account.
A reviewer has been assigned to your package. Learn what to expect during the review process.
IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
@nturaga hi, I have just pushed a new commit to the master branch and also saw a new ping in the webhook response history. But here it shows nothing new. Is it normal?
Received a valid push; starting a build. Commits are:
671a38c update version number
@nturaga I got it now. I've thought changing from "1.0.0" back to "0.99.0" is already a valid version number change. I'm sorry if I disturbed you!
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
Received a valid push; starting a build. Commits are:
6d9cec2 replaced deprecated fn OmaDB::getData by OmaDB::ge...
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
Received a valid push; starting a build. Commits are:
44678fc fixed syntax of OmaDB::getProtein
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
Received a valid push; starting a build. Commits are:
814d0dc fixed syntax of OmaDB::getOMAGroup
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
@nturaga sorry for disturbing you, but for this warning I cannot find a solution. WARNING: Use Authors@R (preferred) or Author/Maintainer fields not both.
I've already updated the DESCRIPTION file in order to remove the Maintainer field, but the warning still exists. Could you please help me on this issue? Many thanks!
This is an issue on our end. Please make other changes if you need to improve your package, once you are ready for a review leave a not saying you are. I'll review your package.
Best,
Nitesh
On Tue, Mar 19, 2019 at 9:39 PM Vinh Tran notifications@github.com wrote:
@nturaga https://github.com/nturaga sorry for disturbing you, but for this warning I cannot find a solution. WARNING: Use Authors@R (preferred) or Author/Maintainer fields not both.
I've already updated the DESCRIPTION file in order to remove the Maintainer field, but the warning still exists. Could you please help me on this issue? Many thanks!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/1039#issuecomment-474447989, or mute the thread https://github.com/notifications/unsubscribe-auth/ACnoSzozvfYaRW1WyKlcM2iVVGK8_3L1ks5vYQvBgaJpZM4bk6fE .
@nturaga Dear Nitesh, I think it is ready for review. Best, Vinh
Please issue another version bump, the warnings should have been resolved.
Received a valid push; starting a build. Commits are:
e9ff67a version bump
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
@nturaga done! :)
General
DESCRIPTION
NEWS
utils::news()
Other
www/
directoryvignettes
R -- Comments are for specific functions but should be applied throughout
stopifnot()
.as.numeric(as.character())
seems like you
are struggle with factor represenation of integer values, why not
insist that the data is integer- (or numeric-) valued up-front, to
avoid this dangerous coercion?clusterProfile.R:96 iterations (for
loops, *apply()
, etc) are
expensive in R, be sure that these are necessary. Even if necessary,
revise your code to 'hoist' common operations out of loops to
minimize the time identical code is evaluated, e.g.,
n <- seq_len(nrow(profiles))
matrix[cbind(n, n)] <- 0
for (i in seq_len(nrow(profiles))) { # rows
p_i <- unlist(profiles[i,])
for (j in seq_len(nrow(profiles))) { # columns
if (i == j)
break
matrix[i, j] <- dcor(p_i, unlist(profiles[j,]))
}
}
# Swich the value so that the profiles with a high correlation
# are clustered together
matrix <- 1 - matrix
message()
to communicate progress to
the user; usually no need to message(paste(...))
, just
message(...)
.rbind()
, here) is
expensive, copying the data frame for each gene. Use
'pre-allocate-and-fill' or lapply()
/ similar to better manage
memory; see
https://bioconductor.org/developers/how-to/efficient-code/ .var1
, var2
used in this function?getFastaSeq.R: 72 no need to re-invent a fasta parser (hmm, but I see you use Biostrings a few lines down?? why implement two different ways of doing the same thing?)
> Biostrings::readAAStringSet(fastaURL)
trying URL 'https://raw.githubusercontent.com/BIONF/phyloprofile-data/master/demo/fastaFile/concatenatedSeq.fa'
Content type 'text/plain; charset=utf-8' length 1275183 bytes (1.2 MB)
==================================================
downloaded 1.2 MB
A AAStringSet instance of length 1931
width seq names
[1] 1376 MDFGGVGRSDRPNRATTLTLLS...APKLGIVPGIAFVSVFVVLLI A.aegypti@7159@16034
[2] 1016 MKLNITSAAGIICLLDEPIQEL...GPKNDDLKEPEAPEPFEYIED A.aegypti@7159@8
[3] 4525 MSGFGTSTGSDLHFRALPYLDK...VLHMNMFRFLGVLMGIAVRTG A.aegypti@7159@5431
[4] 609 MSRNRAAEETDKQTRIAIVSSD...TNSVKDTEQKRAGQYFFYEES A.aegypti@7159@9350
[5] 342 MTLPSSARVYADVNSHKPREYW...FAVIVNGQMPPPSSSTKGSNQ A.aegypti@7159@11472
... ... ...
[1927] 307 MRNLLTDSFELGKRGQSSGNVD...IILLIIILIIILSLKPWSWGK Z.mays@4577@268479
[1928] 1007 MAAVATVSSASGLLAMLQEPAP...GSSAMAVDDEPQPPQPFEYSS Z.mays@4577@228614
[1929] 418 MWRATRRSPPFLRWLSSDAASS...VHVSCGFNHTGAIYECSEDFD Z.mays@4577@298043
[1930] 604 MAERLTRIAIVSEDKCKPKKCR...KLESTKDREQKSAGSYYYLDD Z.mays@4577@306796
[1931] 332 MSKARVYTDVNVLRPKEYWDYE...EAMTHPYFQQVRAAENSRTRA Z.mays@4577@271955
also, please clarify the use of this data -- it is not appropriate to host 1/2 of your package in github, 1/2 in Bioconductor.
paste0()
outside the loop, to avoid unnecessary
iteration & function calls.reverse,character-method
is implemented in
IRanges (already an implicit dependence through Biostrings).pos <- strsplit(domains$location, ":")
start <- vapply(pos, '[[', character(1), 1)
end <- vapply(pos, '[[', character(1), 2)
feature <- ifelse(
nchar(domains$name > 0),
paste0(domains$source, "_", ...),
paste0(domains$source, "_", domains$domainid)
)
feature <- gsub("#", "-", feature)
data.frame(start, end, feature)
feature <- sub("#.*", "", domainList)
data.frame(
seedID = domainID, # why not name function argument `seedID` ?
orthoID,
length,
feature,
start = NA_integer_,
end = NA_integer_,
stringsAsFactors = FALSE
)
read.csv()
ever return an object that
is _not_a data.frame?paste0()
ever generate a warning that
requires supression?cidx <- seq_len(ncol(finalDf) - 3)
colnames(finalDf)[cidx + 3] <- paste0("value_", cidx)
man
other
Please revise your package in response to the comments above, and post a brief summary here when ready for the next review.
Hi @trvinh
If your package is to make the next release, (the deadline is tomorrow) I suggest you start pushing changes now allow a clean build report and also post a response to the comments issued by @mtmorgan.
If not, we are happy for you to continue development at a pace you are comfortable with, and it will go in the next devel cycle (which is perfectly fine).
Another reminder, once a review is given, we give maintainers about 2 weeks to respond. Before we close the issue and it can be reopened once the maintainer has more time to proceed with active development of the package.
Best,
Nitesh
Hi @nturaga I am still doing the revision according to comments of @mtmorgan but I am not sure if I can make it on time for the upcoming release. Whenever it is ready I will push the changes. Best, Vinh
That sounds good. No need to rush the it.
Best,
Nitesh
On Tue, Apr 23, 2019 at 11:50 AM Vinh Tran notifications@github.com wrote:
Hi @nturaga https://github.com/nturaga I am still doing the revision according to comments of @mtmorgan https://github.com/mtmorgan but I am not sure if I can make it on time for the upcoming release. Whenever it is ready I will push the changes. Best, Vinh
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/1039#issuecomment-485862436, or mute the thread https://github.com/notifications/unsubscribe-auth/AAU6QS5DYV67XYMVRPOCEBLPR4V3BANCNFSM4G4TU7CA .
Received a valid push; starting a build. Commits are:
a729bd9 first revision for bioconductor (#99) * fix bug n...
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
Hi @mtmorgan
first of all thanks so much for your comments! I have made some minor changes according to your comments (details in https://github.com/BIONF/PhyloProfile/pull/99#issue-273019196). However I still need to work on improving the code efficiency (e.g. avoiding loop). I am new to R, therefore it takes me some time to understand how this concept works :-) And I will also revise the man pages as well.
About your last comment, the files in inst/PhyloProfile
are part of the package. It is the Shiny-based GUI version of the package.
According to your comment about hosting the data in both github and bioconductor: since Bioconductor requires a package size smaller than 5MB, I need to store the demo data somewhere else and they just need to be downloaded only when the user uses them. Those data are quite large (they are not used for testing the functions but for demonstrating biological use-cases). I will consider your suggestion to create an ExperimentData package
to host them instead of using github.
Best regards, Vinh
Received a valid push; starting a build. Commits are:
5adbf8b fix bug not plotting barplot in groupComparison; a... b31c044 removed www folder, deleted CODE_OF_CONDUCT, chang... 56d03ac replace NEWS by NEWS.md 9a64cd4 removed PhyloProfile:: 1592ff4 resolved as.numeric from all R/.R file 676c911 resolved as.numeric from all inst/PhyloProfile/R/... a6521e7 removed var1, var2 from getPValues (compareTaxaGro... 812efdd removed as.data.frame from read.csv and read.table 705bb3c removed unnecessary suppressWarnings b478830 modified NEWS and moved it to inst 75e20f9 replaced for-loop by vectorized fn for parseMainIn... 6891538 replaced for-loop by vectorized fn for parseDomain... e9fcab7 replaced for-loop by vectorized fn for getOmaBrows... 360afaf fix bug parsing xml input 041f52a improved speed of getDataForOneOma fn 95374d2 fixed getDataForOneOma fn, replaced for-loops in g... ba4ba08 replaced for-loops in estimateGeneAge fn 3e12a28 fixed bug sorting input taxa 836ba5a speed up functions in createTaxonomyMatrix.R d26a48e speed up getSignificantGene fn 2a4f0b3 checked for-loops in inst/PhyloProfile f4c83dd fixed coding style c607c36 revised man pages 0b55e9f added man for filterProfileData fn 07944bc fixed typo server.R 47b469a reduced code lines 3941b40 added default values and simplified function's par... ae6b346 fixed bug for filtering data using filterProfileDa... 9c322a1 rewrote groupComparison functions df2bd0d added more options to groupComparison.R 73673d8 added example for getSelectedTaxonNames fn f6d63e3 revised server.R and ui.R 357c67b removed CODE OF CONDUCT out of README.md cb6bcda finished groupComparison.R 804f5a4 revised server.R and ui.R next db77479 fixed clustering using distance correlation 4583863 fix bugs bsPopover fd07c54 final revision 7fc8041 Resolved merge conflicts. fbdfa00 Merge branch 'trvinh-master'
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
Received a valid push; starting a build. Commits are:
f788027 removed man/getFastaSeqs.Rd
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
Hi @trvinh
Let me know once you are ready for another review. Highlighting the changes you've made according to mtmorgan's comments.
Best,
Nitesh
Hi @nturaga , I am going to ask you one thing regarding to providing the data as an ExperimentData package (I have adapted the codes according to all comments of @mtmorgan, except that one). I am able to create a data package (not yet posted it into the bioconductor issues) and load the data into this PhyloProfile package. However, until the data package is available in bioconductor, some functions of PhyloProfile still depend on github (since the data package is hosting currently only in github). So my question is, must I now submit the data package first, and after it has been accepted and can be installed from bioconductor, I can continue working on PhyloProfile package to remove the github dependency? Many thanks for your declaration! Best, Vinh
Hi @trvinh
You should be able to follow this guide here for your question.
But you must reach out to @lshep (Lori Shepherd) since it's important that your data is first accepted into the AWS S3 bucket. @lshep will guide you through this process.
Best,
Nitesh
@nturaga Dear Nitesh, thanks for your instruction! I have the created the data package following the link you gave me and already mailed @lshep to ask about the access to AWS S3 (did not get the reply yet, I just wrote her last Friday). I am preparing this package to be ready for using the data from the upcoming data package. I will get back to you whenever it is ready for the next review :-) Best, Vinh
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
Received a valid push; starting a build. Commits are:
38cb792 removed getDomainOnline from man pages; added RCur...
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
AdditionalPackage: https://github.com/BIONF/PhyloProfileData
Hi @trvinh,
Starting build on additional package https://github.com/BIONF/PhyloProfileData.
IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your additional package repository will NOT trigger a new build.
The DESCRIPTION file of this additional package is:
Package: PhyloProfileData
Type: Package
Version: 0.99.0
Date: 2019-05-30
Title: Data package for phylogenetic profile analysis using PhyloProfile tool
Author: Ingo Ebersberger, Arpit Jain, Hannah Mülbaier, Vinh Tran
Maintainer: Vinh Tran <tran@bio.uni-frankfurt.de>
Description: Two experimental datasets to illustrate running and analysing phylogenetic profiles with PhyloProfile package.
URL: https://github.com/BIONF/PhyloProfileData
BugReports: https://github.com/trvinh/PhyloProfileData/issues
License: MIT + file LICENSE
Depends: R (>= 3.6.0)
Encoding: UTF-8
biocViews: ExperimentData, ReproducibleResearch, ExperimentHub
Imports: ExperimentHub, Biostrings
Suggests: knitr
VignetteBuilder: knitr
LazyData: false
NeedsCompilation: yes
RoxygenNote: 6.1.1
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
Received a valid push; starting a build. Commits are:
0655d4b removed dependent demo data from github
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.
Please see the build report for more details.
Received a valid push; starting a build. Commits are:
6e5f205 added ExperimentHub to NAMESPACE
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 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.
I am familiar with the essential aspects of Bioconductor software management, including:
For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.