Contributions icon indicating copy to clipboard operation
Contributions copied to clipboard

Polytect: an automatic clustering and labeling method for multi-color digital PCR data

Open emmachenlingo opened this issue 1 year ago • 20 comments

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

  • Repository: https://github.com/emmachenlingo/Polytect

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:

  • [x] The 'devel' branch for new packages and features.
  • [x] The stable 'release' branch, made available every six months, for bug fixes.
  • [x] Bioconductor version control using Git (optionally via GitHub).

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.

emmachenlingo avatar Jun 19 '24 16:06 emmachenlingo

Dear @emmachenlingo,

The package DESCRIPTION must contain valid biocViews.

The following are not valid biocViews terms and should be removed dPCR If you would like to request a term be added please email the [email protected] mailing list and provide details on why and where in the hierarchy you think it should be added.

Please fix your DESCRIPTION. See current biocViews Please also remember to run BiocCheck::BiocCheck('new-package'=TRUE) on your package before submitting a new issue. BiocCheck will look for other Bioconductor package requirements.

bioc-issue-bot avatar Jun 19 '24 16:06 bioc-issue-bot

Hi @emmachenlingo

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: Polytect
Title: An R package for digital data clustering
Version: 0.99.0
Authors@R: 
    person("Yao", "Chen", , "[email protected]", role = c("aut", "cre"),
 comment = c(ORCID = "0000-0001-8172-3996"))
Description: An automatic clustering and labeling method for multi-color digital PCR data to classify partitions into groups based on subpopulations generated by flowPeaks.
biocViews: dPCR, Clustering
License: MIT
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
Depends: 
    R (>= 3.5.0)
Imports: 
    mvtnorm,
    sn,
    dplyr,
    flowPeaks,
    ggplot2,
    tidyverse,
    cowplot,
    mlrMBO,
    DiceKriging
Suggests: 
    testthat (>= 3.0.0),
    knitr,
    rmarkdown
VignetteBuilder: knitr

bioc-issue-bot avatar Jun 19 '24 16:06 bioc-issue-bot

I have changed the biocViews to an existing one. thanks!

emmachenlingo avatar Jun 19 '24 16:06 emmachenlingo

When I perform a R CMD build it is ERRORing. Please ensure R CMD build, R CMD check and BiocCheck are run without ERROR, Warning, or Notes

 R CMD build Polytect 
* checking for file 'Polytect/DESCRIPTION' ... OK
* preparing 'Polytect':
* checking DESCRIPTION meta-information ... OK
* installing the package to build vignettes
* creating vignettes ... ERROR
--- re-building ‘introduction.Rmd’ using rmarkdown

Quitting from lines 68-81 [flowpeaks and merge] (introduction.Rmd)
Error: processing vignette 'introduction.Rmd' failed with diagnostics:
could not find function "polytect_clust"
--- failed re-building ‘introduction.Rmd’

SUMMARY: processing the following file failed:
  ‘introduction.Rmd’

Error: Vignette re-building failed.
Execution halted

lshep avatar Jun 25 '24 13:06 lshep

Also...

Could you provide a comparison to existing Bioconductor packages with similar functionality in the vignette in addition to the generic comparison statement.

Please remove vignette/introduction.pdf and Polytect_manual.pdf. Vignette pdf and man page compilations are generated automatically when R CMD build is run.

lshep avatar Jun 25 '24 13:06 lshep

May we expect any updates soon?

lshep avatar Aug 05 '24 14:08 lshep

This issue is being closed because there has been no progress for an extended period of time. You may reopen the issue when you have the time to actively participate in the review / submission process. Please also keep in mind that a package accepted to Bioconductor requires a commitment on your part to ongoing maintenance.

Thank you for your interest in Bioconductor.

bioc-issue-bot avatar Aug 12 '24 12:08 bioc-issue-bot

May we expect any updates soon?

Sorry. I got stuck with my last-year PhD project. Is it possible to reopen the issue? I think I have fixed all the mentioned problems. This comes with a paper under review. I will certainly commit this time. Thanks and sorry.

emmachenlingo avatar Aug 29 '24 15:08 emmachenlingo

You'll need to include man pages for your exported functions. It looks like you have roxygen markup in your R files; perhaps you forgot to run devtools::document() and commit the rendered documentation?

lshep avatar Sep 03 '24 16:09 lshep

You'll need to include man pages for your exported functions. It looks like you have roxygen markup in your R files; perhaps you forgot to run devtools::document() and commit the rendered documentation?

I have added the "man" pages. Thanks for pointing this out!!!

emmachenlingo avatar Sep 06 '24 11:09 emmachenlingo

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

bioc-issue-bot avatar Sep 17 '24 17:09 bioc-issue-bot

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 24.04.1 LTS): Polytect_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 [email protected]:packages/Polytect to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Sep 17 '24 17:09 bioc-issue-bot

Received a valid push on git.bioconductor.org; starting a build for commit id: 5263c6ff892766438502492d694fbfea9df67810

bioc-issue-bot avatar Oct 02 '24 10:10 bioc-issue-bot

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 24.04.1 LTS): Polytect_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 [email protected]:packages/Polytect to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Oct 02 '24 10:10 bioc-issue-bot

Received a valid push on git.bioconductor.org; starting a build for commit id: ba8708b5e2207945163c6ec77e705ed3bdad18ec

bioc-issue-bot avatar Oct 02 '24 11:10 bioc-issue-bot

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 24.04.1 LTS): Polytect_0.99.2.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 [email protected]:packages/Polytect to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Oct 02 '24 11:10 bioc-issue-bot

A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.

bioc-issue-bot avatar Oct 07 '24 12:10 bioc-issue-bot

Package 'Polytect' Review

The package passed check and build. However there are several things need to be fixed. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.

The NAMESPACE file

  • [ ] Important: Selective imports using importFrom instead of import all with import.
    • in line 9 import(DiceKriging)
    • in line 10 import(ParamHelpers)
    • in line 11 import(cowplot)
    • in line 13 import(flowPeaks)
    • in line 16 import(lhs)
    • in line 17 import(mlrMBO)
    • in line 18 import(mvtnorm)
    • in line 19 import(rgenoud)
    • in line 20 import(smoof)
    • in line 21 import(sn)

R code

  • [ ] Important: warning, message, stop instead of cat and print outside of show methods.
    • In file R/internal_functions.R:
      • at line 422 found ' print(j)'
    • In file R/polytect_clust.R:
      • at line 59 found ' return(print("The parameters of flowPeaks were specified wrong."))'
  • [ ] Important: 1:n is not suggested in source code. Use seq_along, seq_len or seq.int instead.
    • In file R/polytect_clust.R:
      • at line 69 found ' fp_parse$cluster<-fp$peaks.cluster[1:nrow(data_scaled)]'
      • at line 72 found ' data_input<-data_input[1:nrow(data_scaled),]'
      • at line 73 found ' result<-HMM_merge(data_input,cluster_num=cluster_num,base_clust=fp_parse,eps=10^(-10),max_iter=1000,lambdas=lambdas[1:(cluster_num-log2(cluster_num))],coefs=coefs[1:log2(cluster_num)])'
      • at line 76 found ' new_group <- recode(fp_parse$cluster, !!!setNames(result_class, 1:length(g_clusternum)))'
      • at line 81 found ' new_column_names <- c(paste0("channel", 1:(length(column_names) - 1)), column_names[length(column_names)])'
    • In file R/polytect_merge.R:
      • at line 48 found ' summarise(across(1:(ncol(df_data)-1), mean, na.rm = TRUE))'
      • at line 53 found ' result<-HMM_merge(data_input,cluster_num=cluster_num,base_clust=base_clust,eps=10^(-10),max_iter=1000,lambdas=lambdas[1:(cluster_num-log2(cluster_num))],coefs=coefs[1:log2(cluster_num)])'
      • at line 58 found ' new_group = recode(base_clust$cluster, !!!setNames(result_class, 1:length(g_clusternum)))'
      • at line 64 found ' new_column_names <- c(paste0("channel", 1:(length(column_names) - 1)), column_names[length(column_names)])'
  • [ ] NOTE: Vectorize: for loops present, try to replace them by *apply funcitons.
    • In file R/internal_functions.R:
      • at line 57 found ' for (i in seq_along(uclust)) {'
      • at line 69 found ' for (i in seq_along(uclust)) {'
      • at line 156 found ' for (i in comb_seq){'
      • at line 160 found ' for (j in seq_len(comb_ncol)){'
      • at line 196 found ' for (i in seq_len(length(g_clusternum))) {'
      • at line 253 found ' for (l in seq_len(cluster_num)) {'
      • at line 285 found ' for (g in seq_len(length(g_clusternum))) {'
      • at line 286 found ' for (k in seq_len(cluster_num)) {'
      • at line 319 found ' for (g in seq_along(g_clusternum)) {'
      • at line 320 found ' for (k in seq_len(cluster_num)) {'
      • at line 333 found ' for (i in primary_seq){'
      • at line 345 found ' for (j in seq_along(nonprimary_seq)) {'
      • at line 355 found ' for (h in seq_len(cluster_num)){'
      • at line 382 found ' for (k in seq_len(cluster_num)) {'
      • at line 383 found ' for (g in seq_len(length(g_clusternum))) {'
      • at line 421 found ' for (j in seq_len(max_iter)) {'
      • at line 472 found ' for (i in comb_seq){'
      • at line 476 found ' for (j in seq_len(comb_ncol)){'
    • In file R/polytect_plot.R:
      • at line 23 found ' for (i in seq_len(col_num-2)){'
      • at line 25 found ' for (j in col_seq){'
      • at line 54 found ' for (i in seq_len(k)) {'
  • [ ] Important: Remove unused code.
    • In file R/internal_functions.R:
      • at line 11 found ' # si <- silhouette(clustering, dist(data, "euclidean"))'
  • [ ] NOTE: Avoid using '=' for assignment and use '<-' instead. Please user styler package to reformat your package.
    • In file R/polytect_plot.R:
      • at line 26 found ' k=k+1'
    • R/polytect_merge.R (line 58, column 15)
  • [ ] To avoid the * NOTE: Avoid redundant 'stop' and 'warn*' in signal conditions in BioCheck, could you please change or remove the word warning, or error to other word in - R/internal_functions.R (line 424, column 21) - R/internal_functions.R (line 450, column 17)
  • [ ] NOTE: Functional programming: code repetition.
    • repetition in cluster_selection, and combined_vectors
      • in cluster_selection
        • line 6: coef_tmp <- rep(0, primary_tar)
        • line 7: for (i in comb_seq) {
        • line 8: comb_tmp <- combn(seq_len(primary_tar), i)
        • line 9: all_combinations[[i]] <- comb_tmp
        • line 10: comb_ncol <- ncol(comb_tmp)
        • line 11: for (j in seq_len(comb_ncol)) {
        • line 12: mat_coef_tmp <- coef_tmp
        • line 13: mat_coef_tmp[comb_tmp[, j]] <- 1
        • line 14: mat_coef <- rbind(mat_coef, mat_coef_tmp)
        • line 15: }
        • line 16: }
      • in combined_vectors
        • line 12: coef_tmp <- c(1, rep(0, primary_tar))
        • line 13: for (i in comb_seq) {
        • line 14: comb_tmp <- combn(seq_len(primary_tar), i)
        • line 15: all_combinations[[i]] <- comb_tmp
        • line 16: comb_ncol <- ncol(comb_tmp)
        • line 17: for (j in seq_len(comb_ncol)) {
        • line 18: mat_coef_tmp <- coef_tmp
        • line 19: mat_coef_tmp[comb_tmp[, j] + 1] <- (-1)
        • line 20: mat_coef <- rbind(mat_coef, mat_coef_tmp)
        • line 21: }
        • line 22: }
    • repetition in polytect_clust, and polytect_merge
      • in polytect_clust
        • line 3:{
        • line 4: data_scaled <- apply(data, 2, function(x) (x - min(x))/(max(x) -
        • line 5: min(x)))
        • line 6: data_input <- as.matrix(data_scaled)
        • line 42: result <- HMM_merge(data_input, cluster_num = cluster_num,
        • line 43: base_clust = fp_parse, eps = 10^(-10), max_iter = 1000,
        • line 44: lambdas = lambdas[1:(cluster_num - log2(cluster_num))],
        • line 45: coefs = coefs[1:log2(cluster_num)])
        • line 46: result_class <- apply(result[[1]], 1, which.max)
        • line 47: new_group <- recode(fp_parse$cluster, !!!setNames(result_class,
        • line 48: 1:length(g_clusternum)))
        • line 49: df_data <- cbind(data, cluster = new_group)
        • line 50: column_names <- colnames(df_data)
        • line 51: new_column_names <- c(paste0("channel", 1:(length(column_names) -
        • line 52: 1)), column_names[length(column_names)])
        • line 53: colnames(df_data) <- new_column_names
        • line 54: return(as.data.frame(df_data))
      • in polytect_merge
        • line 3:{
        • line 4: data_scaled <- apply(data, 2, function(x) (x - min(x))/(max(x) -
        • line 5: min(x)))
        • line 6: data_input <- as.matrix(data_scaled)
        • line 12: result <- HMM_merge(data_input, cluster_num = cluster_num,
        • line 13: base_clust = base_clust, eps = 10^(-10), max_iter = 1000,
        • line 14: lambdas = lambdas[1:(cluster_num - log2(cluster_num))],
        • line 15: coefs = coefs[1:log2(cluster_num)])
        • line 16: result_class <- apply(result[[1]], 1, which.max)
        • line 17: new_group = recode(base_clust$cluster, !!!setNames(result_class,
        • line 18: 1:length(g_clusternum)))
        • line 19: df_data <- cbind(data, cluster = new_group)
        • line 20: column_names <- colnames(df_data)
        • line 21: new_column_names <- c(paste0("channel", 1:(length(column_names) -
        • line 22: 1)), column_names[length(column_names)])
        • line 23: colnames(df_data) <- new_column_names
        • line 24: return(as.data.frame(df_data))
    • repetition in polytect_summary, and sil_plot
      • in polytect_summary
        • line 1:{
        • line 2: data_scaled <- apply(df_data[, -ncol(df_data)], 2, function(x) (x -
        • line 3: min(x))/(max(x) - min(x)))
        • line 4: data_input <- as.matrix(data_scaled)
        • line 5: df_data2 <- data.frame(cbind(data_input, cluster = df_data$cluster))
        • line 6: sil_coefs <- silhouette_coef(df_data2, df_data2$cluster)
      • in sil_plot
        • line 1:{
        • line 2: data_scaled <- apply(df_data[, -ncol(df_data)], 2, function(x) (x -
        • line 3: min(x))/(max(x) - min(x)))
        • line 4: data_input <- as.matrix(data_scaled)
        • line 5: df_data2 <- data.frame(cbind(data_input, cluster = df_data$cluster))
        • line 6: sil_coefs <- silhouette_coef(df_data2, df_data2$cluster)

Documentation

  • [ ] Note: Vignette should use BiocStyle package for formatting.
    • rmd file vignettes/introduction.Rmd
  • [ ] Important: Vignette should have an Installation section.
    • rmd file vignettes/introduction.Rmd
  • [ ] Important: Please include Bioconductor installation instructions using BiocManager.
    • rmd file vignettes/introduction.Rmd
  • [ ] Important: Vignette should be better formated for the codes. Consider shorter lines; many lines are > 80 characters long.
    • rmd file vignettes/introduction.Rmd
  • [ ] Important: images should be saved in the inst folder. see inst
  • [ ] Note: Consider to include a package man page.

jianhong avatar Oct 09 '24 16:10 jianhong

Received a valid push on git.bioconductor.org; starting a build for commit id: de04c945c06a969a99e27b8043e99d7f70810953

bioc-issue-bot avatar Oct 25 '24 14:10 bioc-issue-bot

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 24.04.1 LTS): Polytect_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 [email protected]:packages/Polytect to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Oct 25 '24 14:10 bioc-issue-bot

@emmachenlingo Please fix the error first. Try to reference this doc

jianhong avatar Nov 12 '24 13:11 jianhong

Received a valid push on git.bioconductor.org; starting a build for commit id: a9b01d904f6248e3ceea9da280c7aa8c6cd9647e

bioc-issue-bot avatar Nov 15 '24 21:11 bioc-issue-bot

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 24.04.1 LTS): Polytect_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 [email protected]:packages/Polytect to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Nov 15 '24 21:11 bioc-issue-bot

@emmachenlingo Please try to answer the comments line by line to help me to locate the changes you made. Thank you.

jianhong avatar Nov 26 '24 13:11 jianhong

Package 'Polytect' Review

The package passed check and build. However there are several things need to be fixed. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.

The NAMESPACE file

  • [x] Important: Selective imports using importFrom instead of import all with import.

    • in line 9 import(DiceKriging)
    • in line 10 import(ParamHelpers)
    • in line 11 import(cowplot)
    • in line 13 import(flowPeaks)
    • in line 16 import(lhs)
    • in line 17 import(mlrMBO)
    • in line 18 import(mvtnorm)
    • in line 19 import(rgenoud)
    • in line 20 import(smoof)
    • in line 21 import(sn) Fixed.

R code

  • [ ] Important: warning, message, stop instead of cat and print outside of show methods.

    • In file R/internal_functions.R:

      • at line 422 found ' print(j)'
    • In file R/polytect_clust.R: changed to message.

* at line 59 found '        return(print("The parameters of flowPeaks were specified wrong."))'

changed to stop.

  • [ ] Important: 1:n is not suggested in source code. Use seq_along, seq_len or seq.int instead.

    • In file R/polytect_clust.R:

      • at line 69 found ' fp_parse$cluster<-fp$peaks.cluster[1:nrow(data_scaled)]'
      • at line 72 found ' data_input<-data_input[1:nrow(data_scaled),]'
      • at line 73 found ' result<-HMM_merge(data_input,cluster_num=cluster_num,base_clust=fp_parse,eps=10^(-10),max_iter=1000,lambdas=lambdas[1:(cluster_num-log2(cluster_num))],coefs=coefs[1:log2(cluster_num)])'
      • at line 76 found ' new_group <- recode(fp_parse$cluster, !!!setNames(result_class, 1:length(g_clusternum)))'
      • at line 81 found ' new_column_names <- c(paste0("channel", 1:(length(column_names) - 1)), column_names[length(column_names)])'
    • In file R/polytect_merge.R:

      • at line 48 found ' summarise(across(1:(ncol(df_data)-1), mean, na.rm = TRUE))'
      • at line 53 found ' result<-HMM_merge(data_input,cluster_num=cluster_num,base_clust=base_clust,eps=10^(-10),max_iter=1000,lambdas=lambdas[1:(cluster_num-log2(cluster_num))],coefs=coefs[1:log2(cluster_num)])'
      • at line 58 found ' new_group = recode(base_clust$cluster, !!!setNames(result_class, 1:length(g_clusternum)))'
      • at line 64 found ' new_column_names <- c(paste0("channel", 1:(length(column_names) - 1)), column_names[length(column_names)])' all changed to seq_len or seq_along
  • [ ] NOTE: Vectorize: for loops present, try to replace them by *apply funcitons.

    • In file R/internal_functions.R:

      • at line 57 found ' for (i in seq_along(uclust)) {'
      • at line 69 found ' for (i in seq_along(uclust)) {'
      • at line 156 found ' for (i in comb_seq){'
      • at line 160 found ' for (j in seq_len(comb_ncol)){'
      • at line 196 found ' for (i in seq_len(length(g_clusternum))) {'
      • at line 253 found ' for (l in seq_len(cluster_num)) {'
      • at line 285 found ' for (g in seq_len(length(g_clusternum))) {'
      • at line 286 found ' for (k in seq_len(cluster_num)) {'
      • at line 319 found ' for (g in seq_along(g_clusternum)) {'
      • at line 320 found ' for (k in seq_len(cluster_num)) {'
      • at line 333 found ' for (i in primary_seq){'
      • at line 345 found ' for (j in seq_along(nonprimary_seq)) {'
      • at line 355 found ' for (h in seq_len(cluster_num)){'
      • at line 382 found ' for (k in seq_len(cluster_num)) {'
      • at line 383 found ' for (g in seq_len(length(g_clusternum))) {'
      • at line 421 found ' for (j in seq_len(max_iter)) {'
      • at line 472 found ' for (i in comb_seq){'
      • at line 476 found ' for (j in seq_len(comb_ncol)){'
    • In file R/polytect_plot.R:

      • at line 23 found ' for (i in seq_len(col_num-2)){'
      • at line 25 found ' for (j in col_seq){'
      • at line 54 found ' for (i in seq_len(k)) {' In progress. Many functions are based on long mathematical formula, hard to replace it with apply.
  • [ ] Important: Remove unused code.

    • In file R/internal_functions.R:

      • at line 11 found ' # si <- silhouette(clustering, dist(data, "euclidean"))' removed.
  • [ ] NOTE: Avoid using '=' for assignment and use '<-' instead. Please user styler package to reformat your package.

    • In file R/polytect_plot.R:

      • at line 26 found ' k=k+1'
    • R/polytect_merge.R (line 58, column 15) changed to '<-'

  • [ ] To avoid the * NOTE: Avoid redundant 'stop' and 'warn*' in signal conditions in BioCheck, could you please change or remove the word warning, or error to other word in
    • R/internal_functions.R (line 424, column 21)
    • R/internal_functions.R (line 450, column 17) changed to 'Note'.
  • [ ] NOTE: Functional programming: code repetition.

    • repetition in cluster_selection, and combined_vectors

      • in cluster_selection

        • line 6: coef_tmp <- rep(0, primary_tar)
        • line 7: for (i in comb_seq) {
        • line 8: comb_tmp <- combn(seq_len(primary_tar), i)
        • line 9: all_combinations[[i]] <- comb_tmp
        • line 10: comb_ncol <- ncol(comb_tmp)
        • line 11: for (j in seq_len(comb_ncol)) {
        • line 12: mat_coef_tmp <- coef_tmp
        • line 13: mat_coef_tmp[comb_tmp[, j]] <- 1
        • line 14: mat_coef <- rbind(mat_coef, mat_coef_tmp)
        • line 15: }
        • line 16: }
      • in combined_vectors

        • line 12: coef_tmp <- c(1, rep(0, primary_tar))
        • line 13: for (i in comb_seq) {
        • line 14: comb_tmp <- combn(seq_len(primary_tar), i)
        • line 15: all_combinations[[i]] <- comb_tmp
        • line 16: comb_ncol <- ncol(comb_tmp)
        • line 17: for (j in seq_len(comb_ncol)) {
        • line 18: mat_coef_tmp <- coef_tmp
        • line 19: mat_coef_tmp[comb_tmp[, j] + 1] <- (-1)
        • line 20: mat_coef <- rbind(mat_coef, mat_coef_tmp)
        • line 21: }
        • line 22: }
    • repetition in polytect_clust, and polytect_merge

      • in polytect_clust

        • line 3:{
        • line 4: data_scaled <- apply(data, 2, function(x) (x - min(x))/(max(x) -
        • line 5: min(x)))
        • line 6: data_input <- as.matrix(data_scaled)
        • line 42: result <- HMM_merge(data_input, cluster_num = cluster_num,
        • line 43: base_clust = fp_parse, eps = 10^(-10), max_iter = 1000,
        • line 44: lambdas = lambdas[1:(cluster_num - log2(cluster_num))],
        • line 45: coefs = coefs[1:log2(cluster_num)])
        • line 46: result_class <- apply(result[[1]], 1, which.max)
        • line 47: new_group <- recode(fp_parse$cluster, !!!setNames(result_class,
        • line 48: 1:length(g_clusternum)))
        • line 49: df_data <- cbind(data, cluster = new_group)
        • line 50: column_names <- colnames(df_data)
        • line 51: new_column_names <- c(paste0("channel", 1:(length(column_names) -
        • line 52: 1)), column_names[length(column_names)])
        • line 53: colnames(df_data) <- new_column_names
        • line 54: return(as.data.frame(df_data))
      • in polytect_merge

        • line 3:{
        • line 4: data_scaled <- apply(data, 2, function(x) (x - min(x))/(max(x) -
        • line 5: min(x)))
        • line 6: data_input <- as.matrix(data_scaled)
        • line 12: result <- HMM_merge(data_input, cluster_num = cluster_num,
        • line 13: base_clust = base_clust, eps = 10^(-10), max_iter = 1000,
        • line 14: lambdas = lambdas[1:(cluster_num - log2(cluster_num))],
        • line 15: coefs = coefs[1:log2(cluster_num)])
        • line 16: result_class <- apply(result[[1]], 1, which.max)
        • line 17: new_group = recode(base_clust$cluster, !!!setNames(result_class,
        • line 18: 1:length(g_clusternum)))
        • line 19: df_data <- cbind(data, cluster = new_group)
        • line 20: column_names <- colnames(df_data)
        • line 21: new_column_names <- c(paste0("channel", 1:(length(column_names) -
        • line 22: 1)), column_names[length(column_names)])
        • line 23: colnames(df_data) <- new_column_names
        • line 24: return(as.data.frame(df_data))
    • repetition in polytect_summary, and sil_plot

      • in polytect_summary

        • line 1:{
        • line 2: data_scaled <- apply(df_data[, -ncol(df_data)], 2, function(x) (x -
        • line 3: min(x))/(max(x) - min(x)))
        • line 4: data_input <- as.matrix(data_scaled)
        • line 5: df_data2 <- data.frame(cbind(data_input, cluster = df_data$cluster))
        • line 6: sil_coefs <- silhouette_coef(df_data2, df_data2$cluster)
      • in sil_plot

        • line 1:{
        • line 2: data_scaled <- apply(df_data[, -ncol(df_data)], 2, function(x) (x -
        • line 3: min(x))/(max(x) - min(x)))
        • line 4: data_input <- as.matrix(data_scaled)
        • line 5: df_data2 <- data.frame(cbind(data_input, cluster = df_data$cluster))
        • line 6: sil_coefs <- silhouette_coef(df_data2, df_data2$cluster) In progress.

Documentation

  • [ ] Note: Vignette should use BiocStyle package for formatting. In progress
  • rmd file vignettes/introduction.Rmd

  • [x] Important: Vignette should have an Installation section.

    • rmd file vignettes/introduction.Rmd added.
  • [ ] Important: Please include Bioconductor installation instructions using BiocManager.

    • rmd file vignettes/introduction.Rmd added
  • [ ] Important: Vignette should be better formated for the codes. Consider shorter lines; many lines are > 80 characters long.

    • rmd file vignettes/introduction.Rmd code lines have been shortened less than 80 characters.
  • [ ] Important: images should be saved in the inst folder. see inst moved the images to inst folder
  • [ ] Note: Consider to include a package man page. in progress

emmachenlingo avatar Dec 09 '24 14:12 emmachenlingo

I have fixed all the 'important' issues and part of 'Note'. The replies are highlighted in italic and bold. Thanks!!!

emmachenlingo avatar Dec 09 '24 14:12 emmachenlingo

Package 'Polytect' Review

The package passed check and build. It is almost there. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.

General package development

  • [ ] Important: Consider adding input checking. We strongly encourage them. See https://contributions.bioconductor.org/r-code.html#function-arguments

jianhong avatar Dec 16 '24 15:12 jianhong

Received a valid push on git.bioconductor.org; starting a build for commit id: ff9290d10fa6c4ec0b74fc98ec3d19bb08e99e6a

bioc-issue-bot avatar Dec 27 '24 22:12 bioc-issue-bot

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 24.04.1 LTS): Polytect_0.99.5.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 [email protected]:packages/Polytect to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Dec 27 '24 22:12 bioc-issue-bot

Package 'Polytect' Review

The package passed check and build. It is almost there. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.

General package development

  • [ ] Important: Consider adding input checking. We strongly encourage them. See https://contributions.bioconductor.org/r-code.html#function-arguments

I have added input checking for each external function. Thanks very much! And happy holidays!

emmachenlingo avatar Dec 27 '24 23:12 emmachenlingo