Contributions icon indicating copy to clipboard operation
Contributions copied to clipboard

iDA submission

Open reesyxan opened this issue 3 years ago • 18 comments

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

  • Repository: https://github.com/reesyxan/iDA

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.

reesyxan avatar Aug 02 '22 15:08 reesyxan

Hi @reesyxan

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: iDA
Type: Package
Title: Define embedding space for data with clustered architecture
Version: 0.99.0
Authors@R: c(person("Theresa", "Alexander", 
    email = "[email protected]", role = "cre"), 
    person("Hector", "Corrada Bravo", 
    email = "[email protected]", role = "aut"))
Imports: scran, igraph, irlba, SingleCellExperiment, NetworkToolbox, DESeq2, 
    SummarizedExperiment, mclust, stats, scuttle, genefilter, dplyr, utils, 
    airway, S4Vectors, plyr, methods
Description: Discovering low-dimensional embeddings that describe the separation 
    of any underlying discrete latent structure in data is an important 
    motivation for applying these techniques since these latent classes can 
    represent important sources of unwanted variability, such as batch effects,
    or interesting sources of signal such as unknown cell types. The features 
    that define this discrete latent structure are often hard to identify in
    high-dimensional data. Principal component analysis (PCA) is one of the most
    widely used methods as an unsupervised step for dimensionality reduction. 
    This reduction technique finds linear transformations of the data which 
    explain total variance. When the goal is detecting discrete structure, PCA 
    is applied with the assumption that classes will be separated in directions 
    of maximum variance. However, PCA will fail to accurately find discrete 
    latent structure if this assumption does not hold. Visualization techniques,
    such as t-Distributed Stochastic Neighbor Embedding (t-SNE) and Uniform
    Manifold Approximation and Projection (UMAP), attempt to mitigate these 
    problems with PCA by creating a low-dimensional space where similar objects 
    are modeled by nearby points in the low-dimensional embedding and dissimilar
    objects are modeled by distant points with high probability. However, since
    t-SNE and UMAP are computationally expensive, often a PCA reduction is done 
    before applying them which makes it sensitive to PCAs downfalls. Also, tSNE 
    is limited to only two or three dimensions as a visualization tool, which 
    may not be adequate for retaining discriminatory information. The linear 
    transformations of PCA are preferable to non-linear transformations provided
    by methods like t-SNE and UMAP for interpretable feature weights. Here, we 
    propose iterative discriminant analysis (iDA), a dimensionality reduction 
    technique designed to mitigate these limitations. iDA takes in a scaled data
    matrix of variable features and outputs an embedding that carries 
    discriminatory information which optimally separates latent clusters using
    linear transformations that permit post hoc analysis to determine features 
    that define these latent structures.
License: MIT + file LICENSE
VignetteBuilder: knitr
Encoding: UTF-8
RoxygenNote: 7.1.2
Suggests: 
    testthat, 
    datasets, 
    BiocStyle, 
    knitr, 
    rmarkdown,
    scPipe,
    scater,
    ggplot2,
    Rtsne,
    celldex,
    scRNAseq
biocViews: Software, DimensionReduction, Clustering

bioc-issue-bot avatar Aug 02 '22 15:08 bioc-issue-bot

Does your package build on R 4.2/Bioc 3.16?

> start <- Sys.time()

> sce <- iDA(sce)
1000 variable features found using scran::getTopHVGs. 

Error in (t(i - clustermeans[[k]])) %*% (i - clustermeans[[k]]) : 
  requires numeric/complex matrix/vector arguments

Enter a frame number, or 0 to exit   

1: source("iDAvignette.R", echo = TRUE)
2: withVisible(eval(ei, envir))
3: eval(ei, envir)
4: eval(ei, envir)
5: iDAvignette.R#43: iDA(sce)
6: iDA(sce)
7: .local(object, ...)
8: .iDA_core(var.data, ...)
9: WCS(splitclusters = splitclusters, diag = diag)

vjcitn avatar Aug 05 '22 03:08 vjcitn

Hello,

The error is fixed now.

reesyxan avatar Aug 07 '22 19:08 reesyxan

A reviewer has been assigned to your package. Learn what to expect during the review process.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

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 Aug 26 '22 12:08 bioc-issue-bot

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. This link will be 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/iDA to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Aug 26 '22 12:08 bioc-issue-bot

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

bioc-issue-bot avatar Sep 09 '22 15:09 bioc-issue-bot

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. This link will be 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/iDA 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 09 '22 15:09 bioc-issue-bot

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

bioc-issue-bot avatar Sep 09 '22 15:09 bioc-issue-bot

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. This link will be 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/iDA 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 09 '22 15:09 bioc-issue-bot

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

bioc-issue-bot avatar Sep 12 '22 14:09 bioc-issue-bot

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. This link will be 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/iDA 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 12 '22 15:09 bioc-issue-bot

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

bioc-issue-bot avatar Sep 12 '22 15:09 bioc-issue-bot

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, skipped". 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. This link will be 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/iDA 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 12 '22 15:09 bioc-issue-bot

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

bioc-issue-bot avatar Sep 19 '22 13:09 bioc-issue-bot

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. This link will be 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/iDA 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 19 '22 13:09 bioc-issue-bot

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

bioc-issue-bot avatar Sep 19 '22 15:09 bioc-issue-bot

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. This link will be 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/iDA 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 19 '22 16:09 bioc-issue-bot

Hi @reesyxan

Thanks for submitting iDA :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

Review

Key: :rotating_light: Required :warning: Recommended :green_circle: Optional :question: Questionl

General package development

  • [ ] :warning: There are some notes from BiocCheck::BiocCheck() in the build report, please address as many of these as possible
  • [ ] :warning: I got a note from R CMD check about {gridExtra} needing to be added as a suggested dependency, it doesn't seem to be on the main build report though

DESCRIPTION file

  • [ ] :warning: The Description field is quite long and has a lot of general introduction, have a look at the advice here and here
  • [ ] :warning: It is recommended to add ORCiDs to the Authors@R field
  • [ ] :warning: It is recommended to have a BugReports field, usually this is a link to your GitHub issues page or the Bioconductor support forum
  • [ ] :warning: It is recommended to have a URL field, usually this is a link to your GitHub page or the Bioconductor package page

NAMESPACE file

  • [ ] :rotating_light: In general you should use importFrom instead of import all with import

The NEWS file

  • [ ] :rotating_light: You currently have a Markdown NEWS.md file but you use R documentation style formatting in it, the formatting and the file type should match

The CITATION file

  • [ ] :warning: You mention a publication in the vignette, consider adding a proper CITATION file with this information

Documentation

README

  • [ ] :rotating_light: The images in your README are not linked correctly and aren't shown on the GitHub webpage

Vignette

  • [ ] :rotating_light: Please add a table of contents
  • [ ] :rotating_light: You currently have some unevaluated code blocks, please remove these (except for the installation instructions), some of these also contain invalid code
  • [ ] :rotating_light: Please rename the Overview section to Introduction
    • [ ] :rotating_light: Please add motivation for including the package in Bioconductor (using common objects, interactions with other packages, using annotations etc.)
    • [ ] :rotating_light: Please mention other packages that can be used to perform the same task and how you package compares to them (if applicable)
  • [ ] :green_circle: You have some code chunks that just print timing information, I would suggest either combining this with the function code or incorporating them into the text using inline code
  • [ ] :green_circle: In some places you extract information from the SCE/SE as a new variable and use it for things like computing a t-SNE, I would suggest using existing functions (for example from the {scuttle}/{scater} packages) that operate on these objects directly instead

Man pages

  • [ ] :warning: It is recommended to add a package man page
  • [ ] :warning: Please check the ?iDA man page, there is some repeated and unrelated information here
  • [ ] :warning: Please check the formatting in your man pages, there are some places where the markup is incorrect and can be seen in the rendered page

Unit tests

  • [ ] :warning: Please consider adding more unit tests, particularly for the different input types (matrix/SCE/SE...)
  • [ ] :warning: The naming for test files should generally match the naming of your .R files

Code

R

  • [ ] :warning: Consider adding validity tests for arguments in all exported functions
  • [ ] :rotating_light: I think some of your for loops could be replaced with apply statements, please check these
  • [ ] :green_circle: Currently you calculate highly variable genes as part of your functions but in current workflows, this is usually done as a separate step before dimensionality reduction. I would strongly suggest modifying your functions to follow this workflow.
    • [ ] :warning: Check that the nfeatures argument is used properly in functions, I think there are some places where there is a hardcoded value

Additional files

  • [ ] :rotating_light: There are some additional files in inst/. I'm not sure what these are for but please remove them if not necessary.

lazappi avatar Sep 30 '22 10:09 lazappi

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 Dec 16 '22 12:12 bioc-issue-bot