search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

corral #1492

Closed laurenhsu1 closed 4 years ago

laurenhsu1 commented 4 years ago

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]'

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.

bioc-issue-bot commented 4 years ago

Hi @laurenhsu1

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: corral
Title: Correspondence Analysis for Single Cell Data
Version: 1.0.0
Date: 2020-02-21
Author: Lauren Hsu & Aedin Culhane
Maintainer: Lauren Hsu <lrnshoe@gmail.com>
Description: Performs and plots correspondence analysis on single and multiple tables of single cell count data.
Imports: ggplot2, 
         ggthemes, 
         irlba, 
         Matrix, 
         SingleCellExperiment, 
         transport
Suggests: CellBench,
DuoClustering2018,
knitr,
LaplacesDemon,
scater,
testthat
License: GPL-2
RoxygenNote: 7.1.0
VignetteBuilder: knitr
biocViews: SingleCell, DimensionReduction, Visualization, BatchEffect, PrincipalComponent, Preprocessing
Encoding: UTF-8

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.

bioc-issue-bot commented 4 years ago

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.

mtmorgan commented 4 years ago

Can you please

bioc-issue-bot commented 4 years ago

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.

laurenhsu1 commented 4 years ago

I just updated the description, removed the html files, and bumped the version.

LiNk-NY commented 4 years ago

Hi Lauren, @laurenhsu1 There are some errors in the build. Remove any *.Rproj files

laurenhsu1 commented 4 years ago

Hi Marcel, I have removed the Rproj file.

LiNk-NY commented 4 years ago

Can you bump the package z version and push? Thanks.

aedin commented 4 years ago

Marcel should the version number be 0.99.1 or 1.0.1? Also can you advise on SummarizedExperiment and MAE error.

laurenhsu1 commented 4 years ago

I already did, in these two commits: https://github.com/laurenhsu1/corral/commit/3831c98f3f9516e9af742ec00553d521ba3bfe3b https://github.com/laurenhsu1/corral/commit/a837a3f17e10a52867c3872292d175fb7e707b1c Do I need to bump it again?

LiNk-NY commented 4 years ago

Hi Aedin @aedin and Lauren @laurenhsu1,

The version number should be 0.99.1 where 1 is the z version in x.y.z. Version bumps are z + 1.

At least for MultiAssayExperiment, you don't need to use @importClassesFrom since you are not creating a class that inherits from MultiAssayExperiment (AFAICT from the peek that I took). If you are using other functions, use the @importFrom for those functions, i.e., assay, assays, experiments, intersectRows.

I think the "bump" was a version regression as you had already published 1.0. It might be tripping up the SPB. I can check with Lori and/or Kayla. @lshep @Kayla-Morrell Thanks.

LiNk-NY commented 4 years ago

@laurenhsu1 Please add DuoClustering2018 to the Suggests field in the DESCRIPTION.

> library(DuoClustering2018)
Error in library(DuoClustering2018) :
  there is no package called ‘DuoClustering2018’
laurenhsu1 commented 4 years ago

Hi Marcel @LiNk-NY, it is there: https://github.com/laurenhsu1/corral/blob/master/DESCRIPTION

LiNk-NY commented 4 years ago

Hi Lauren, @laurenhsu1 Sorry! It's not installed in my system.

Can you please use 2 or 4 spaces in the DESCRIPTION file and add indentation to the Description: field?

We usually download packages like this: 

> BiocManager::install("laurenhsu1/corral", dependencies = TRUE)
Bioconductor version 3.12 (BiocManager 1.30.10), R 4.0.0 beta (2020-04-14
  r78225)
Installing github package(s) 'laurenhsu1/corral'
Using github PAT from envvar GITHUB_PAT
Error: Failed to install 'unknown package' from GitHub:
  Line starting 'similar to principal ...' is malformed!

but currently, I'm getting an error. It is due to the formatting in the DESCRIPTION file. Thanks!

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

a7b8a18 description spacing + print method updates

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

c7830e7 version bump for namespace update

aedin commented 4 years ago

Hi Marcel I tested BiocManager::install("laurenhsu1/corral", dependencies = TRUE) on my Mac and it worked ok. Hopefully, it works for you now also Thanks A

bioc-issue-bot commented 4 years ago

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.

bioc-issue-bot commented 4 years ago

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.

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

1d6bb98 namespace + roxygen updates for importing from MAE... 8e0c4d1 Merge branch 'master' of https://github.com/lauren...

bioc-issue-bot commented 4 years ago

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.

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

0352171 addressing errors: fixing roxygen example; adding ...

bioc-issue-bot commented 4 years ago

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.

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

4edb4ca fixing examples addressing errors from: https://bi... 35801fb Merge branch 'master' of https://github.com/lauren...

bioc-issue-bot commented 4 years ago

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.

laurenhsu1 commented 4 years ago

Hi Marcel @LiNk-NY ,

In the latest build report (https://bioconductor.org/spb_reports/corral_buildreport_20200502002751.html#tokay2_check_anchor), this is the only error left:

I added it back in, yet it is still giving this error. I also got a different error about it when I didn't include it in the roxygen/Namespace file. Do you have suggestions about what how I should resolve this?

Thanks

LiNk-NY commented 4 years ago

Hi Lauren @laurenhsu1 & Aedin @aedin

It seems to be a weird bug in BiocCheck but there are a few typos on this line: https://github.com/laurenhsu1/corral/blob/35801fb119d72091bb8df36c1001797aa8958f89/R/corralm.R#L118 where : should be ::.

Once this is fixed, it will give you a note: 

* Checking to see if we understand object initialization...
    * NOTE: Consider clarifying how 4 object(s) are initialized. Maybe
      they are part of a data set loaded with data(), or perhaps part
      of an object referenced in with() or within().
    function (object)
      corral (is)
      corral_preproc (is)
      corralm (is)
      get_weights (is)
bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

130739a fixing typo

laurenhsu1 commented 4 years ago

Thanks Marcel @LiNk-NY, I just fixed that line with the single colons

bioc-issue-bot commented 4 years ago

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.

LiNk-NY commented 4 years ago

Hi Lauren, @laurenhsu1 There are still a couple of warnings to resolve: 

* checking package subdirectories ... WARNING
Subdirectory 'inst/doc' contains invalid file names:
  '1-corral.Rmd' '2-corralm.Rmd' '1-corral.html' '2-corralm.html'
Please remove or rename the files.
See section 'Package subdirectories' in the 'Writing R Extensions'
manual.

I guess it means that the - in the file name is not allowed.

These are relatively quick fixes: 

* checking S3 generic/method consistency ... WARNING
print:
  function(x, ...)
print.corral:
  function(inp)

print:
  function(x, ...)
print.corralm:
  function(inp)

See section 'Generic functions and methods' in the 'Writing R
Extensions' manual.
* checking R code for possible problems ... [41s] NOTE
corral: no visible global function definition for 'is'
corral_preproc: no visible global function definition for 'is'
corralm: no visible global function definition for 'is'
get_weights: no visible global function definition for 'is'
Undefined global functions or variables:
  is
Consider adding
  importFrom("methods", "is")
to your NAMESPACE file (and ensure that your DESCRIPTION Imports field
contains 'methods').

There are couple of other notes in the report as well. Particularly the 100 character note for PDF vignettes (?) and the 'unstated dependencies' note that should be resolved.

I will continue with the review of the package shortly. Best, Marcel

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

43dba94 line lengths of examples; rename vignettes. addres...

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

a26ef90 add methods is into roxygen; rename input for s3 p...

bioc-issue-bot commented 4 years ago

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, TIMEOUT, 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.

bioc-issue-bot commented 4 years ago

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.

laurenhsu1 commented 4 years ago

Hi Marcel @LiNk-NY, I just pushed a version that addresses these, but now addressing this one:

* checking R code for possible problems ... [41s] NOTE
corral: no visible global function definition for 'is'
corral_preproc: no visible global function definition for 'is'
corralm: no visible global function definition for 'is'
get_weights: no visible global function definition for 'is'
Undefined global functions or variables:
  is
Consider adding
  importFrom("methods", "is")
to your NAMESPACE file (and ensure that your DESCRIPTION Imports field
contains 'methods').

from the previous version (https://bioconductor.org/spb_reports/corral_buildreport_20200504112016.html) results in an error creates a new error:

* checking package dependencies ... ERROR
Namespace dependency not required: 'methods'

See section 'The DESCRIPTION file' in the 'Writing R Extensions'
manual.
* DONE

Status: 1 ERROR

in the most recent one (https://bioconductor.org/spb_reports/corral_buildreport_20200504145357.html)

Do you have any suggestions about how to resolve this?

Thanks

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

e037be2 add methods to description

bioc-issue-bot commented 4 years ago

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.

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

6a6f6fd updating s3 print roxygen addressing warnings/not...

bioc-issue-bot commented 4 years ago

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.

LiNk-NY commented 4 years ago

Hi Lauren, @laurenhsu1

Thank you for your submission. Please see the review below. If you have any questions, feel free to post them as comments below.

Best, Marcel

corral #1492

DESCRIPTION

NAMESPACE

vignettes/

R

test

aedin commented 4 years ago

Thanks Lauren has her thesis defense on a Thursday so probably won’t review this until after that.

It would be great to have a quick discussion (maybe slack or zoom??) on how best to interrelate corral with sce and mae and other bioc classes.

Aedin

Sent from my iPhone

On May 5, 2020, at 7:47 PM, Marcel Ramos notifications@github.com wrote:

 External Email - Use Caution

Hi Lauren, @laurenhsu1

Thank you for your submission. Please see the review below. If you have any questions, feel free to post them as comments below.

Best, Marcel

corral #1492

DESCRIPTION

It looks good. Minor: Use Authors@R instead of Author and Maintainer fields if you'd like to add your ORCID ID and authorship details. Consider using BiocStyle for the vignettes and adding it to Suggests NAMESPACE

Looks good. It seems that you are using an S3 class. vignettes/

Please keep the text within the 80 column width limit. It's hard to read and maintain when the text goes off the page. Perhaps it would be better for users to expect and get a consistent output despite the input (e.g., SingleCellExperiment or a derivative) It would be good to have an easy way to convert a list of SingleCellExperiments or similar into a MultiAssayExperiment object that uses the colData in one motion. The code in the 4th chunk of in the alignment vignette would be avoided. You could then do assays to get a list of matrices. R

Use match.arg within the function instead of c("irl", "svd")[1] for vector default arguments (corralm_matlist) Avoid using 'false' strings. Use FALSE. Shorten error messages where possible. Re-using an S4 class has added benefits of a show method. Perhaps use a List derivative, ExperimentList, etc. Functions that are not intended to be called directly should be internal. Use a @keywords internal tag. Avoid using numeric indices where possible and use more robust names. When creating a sequence, avoid using 1:n syntax, use the more robust seq_along(x) or seq_len(nrow(x)). The na2zero implementation should be endomorphic and vectorized so that it works on a matrix and returns a matrix rather than having the user enter, apply(x, c(1,2), na2zero). If I understand the process correctly, the add_embeddings* function adds the results to the list of SCE objects. Perhaps it supports the idea that the package could use a unified representation of SCEs as a List / ExperimentList. Minor: It would be nice to see a single function handles both single and multiple tables and returns a consistent output. test

Optional: Add more tests to increase the robustness of the package. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

LiNk-NY commented 4 years ago

Hi Aedin, @aedin I can meet today via Slack call. Let me know what time works for you. Best, Marcel

laurenhsu1 commented 4 years ago

Hi Marcel @LiNk-NY, Thanks for your review, now in the process of making updates to the package. Could we schedule the slack call Aedin @aedin suggested at some point tomorrow or Thursday? We are both available in the afternoons those days, please let us know if there's a time that works for you too. Best, Lauren

LiNk-NY commented 4 years ago

Hi Lauren @laurenhsu1 and Aedin @aedin, What times are you available? I can do today and tomorrow after 4 PM EDT (GMT-4) or 12 - 1 PM tomorrow. Best, Marcel

laurenhsu1 commented 4 years ago

Hi Marcel @LiNk-NY, Let's meet today at 4p eastern on the bioconductor slack. Thanks, Lauren

LiNk-NY commented 4 years ago

:ok_hand: see you then!

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

e5f4863 update based on review part 1 of changes for https...

bioc-issue-bot commented 4 years ago

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.

bioc-issue-bot commented 4 years ago

Received a valid push; starting a build. Commits are:

b640f26 update description in response to https://biocondu...