Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sketchR #3284

Closed
10 tasks done
csoneson opened this issue Feb 1, 2024 · 34 comments
Closed
10 tasks done

sketchR #3284

csoneson opened this issue Feb 1, 2024 · 34 comments
Assignees
Labels
3a. accepted will be ingested into Bioconductor daily builder for distribution OK

Comments

@csoneson
Copy link

csoneson commented Feb 1, 2024

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 understand that by submitting my package to Bioconductor,
    the package source and all review commentary are visible to the
    general public.

  • I have read the Bioconductor Package Submission
    instructions. My package is consistent with the Bioconductor
    Package Guidelines.

  • I understand Bioconductor Package Naming Policy and acknowledge
    Bioconductor may retain use of package name.

  • 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.

  • My package addresses statistical or bioinformatic issues related
    to the analysis and comprehension of high throughput genomic data.

  • 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 Bioconductor code of conduct and
    agree to abide by it.

I am familiar with the essential aspects of Bioconductor software
management, including:

  • The 'devel' branch for new packages and features.
  • The stable 'release' branch, made available every six
    months, for bug fixes.
  • 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.

@bioc-issue-bot
Copy link
Collaborator

Hi @csoneson

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: sketchR
Version: 0.99.0
Date: 2024-02-01
Title: An R interface for python subsampling/sketching algorithms
Authors@R: c(
    person("Charlotte", "Soneson", email = "[email protected]",
        role = c("aut", "cre"), comment = c(ORCID = "0000-0003-3833-2169")),
    person("Michael", "Stadler", email = "[email protected]",
 role = c("aut"), comment = c(ORCID = "0000-0002-2269-4934")))
License: MIT + file LICENSE
Description: Provides an R interface for various subsampling algorithms 
    implemented in python packages. Currently, interfaces to the geosketch 
    and scSampler python packages are implemented. In addition it also
    provides diagnostic plots to evaluate the subsampling.
Imports:
    basilisk,
    Biobase,
    DelayedArray,
    dplyr,
    ggplot2,
    methods,
    reticulate,
    rlang,
    scales,
    stats
Suggests:
    rmarkdown,
    knitr,
    testthat (>= 3.0.0),
    TENxPBMCData,
    scuttle,
    scran,
    scater,
    SingleR,
    celldex,
    cowplot,
    SummarizedExperiment,
    beachmat.hdf5,
    BiocStyle,
    BiocManager,
    SingleCellExperiment
URL: https://github.com/fmicompbio/sketchR
BugReports: https://github.com/fmicompbio/sketchR/issues
RoxygenNote: 7.3.1
Encoding: UTF-8
StagedInstall: no
Config/testthat/edition: 3
VignetteBuilder: knitr
biocViews: SingleCell

@bioc-issue-bot bioc-issue-bot added the 1. awaiting moderation submitted and waiting clearance to access resources label Feb 1, 2024
@lshep lshep added the pre-check passed pre-review performed and ready to be added to git label Feb 5, 2024
@bioc-issue-bot
Copy link
Collaborator

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 bioc-issue-bot added pre-review on bioconductor git and access to on demand build but not assigned reviewer until build report clean and removed 1. awaiting moderation submitted and waiting clearance to access resources pre-check passed pre-review performed and ready to be added to git labels Feb 5, 2024
@bioc-issue-bot
Copy link
Collaborator

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

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

@csoneson
Copy link
Author

csoneson commented Feb 5, 2024

I think the timeout here (on macOS only) comes from building the conda environment, which I'm not sure how to do much about - this should only have to be done once, and then things should be much faster. Is there anything that can be done to retrigger the build and let it run through (I guess that if I bump the version it will try to create a new environment)?

@hpages
Copy link
Contributor

hpages commented Feb 8, 2024

Hi Charlotte,

We've made some changes to how basilisk's cache is set up on nebbiolo1 and merida1. Now it's shared between the daily builds (BBS) and SPB. This should help a little bit with R CMD build and R CMD check timings of sketchR because the SPB will now benefit from things that are already installed by the daily builds.

Can you bump the version of the package? We'll see how it goes.

Note that there will still be a substantial amount of stuff that needs to be reinstalled over and over each time you bump the version of the package (this is how basilisk was designed, I can't think of an easy way to avoid that), but the cache also contains a significant amount of stuff that is version-independent. That part won't be reinstalled.

Best

@bioc-issue-bot
Copy link
Collaborator

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

@bioc-issue-bot
Copy link
Collaborator

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

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

@hpages
Copy link
Contributor

hpages commented Feb 8, 2024

hmm... 😞 I'll take a look...

@csoneson
Copy link
Author

csoneson commented Feb 8, 2024

Thanks Hervé! Let me know if there's something else I should try (I can probably reduce the run time of the vignette a little by using a smaller data set to start with, but I'm not sure that has a big impact compared to the time spent building the environment).

@hpages
Copy link
Contributor

hpages commented Feb 8, 2024

Turns out that the SPB was still using the old cache location (~/Library/Caches/org.R-project.R/R/basilisk). Looks like the SPB server needs to be restarted for the new cache location (/var/cache/basilisk) to become effective, so I just did that.

Can you please try again? 🤞

@bioc-issue-bot
Copy link
Collaborator

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

@bioc-issue-bot
Copy link
Collaborator

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

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

@hpages
Copy link
Contributor

hpages commented Feb 9, 2024

Bummer! 😞

FWIW I was able to manually build sketchR in an interactive session on merida1 in 9m32s (see Bioconductor/BBS#400), but that was when the daily builds were not running so I guess that gave me a small advantage.

Anyways, even though sharing the cache seems to help a little bit, it's not enough.

Let me think about what else we can do about this.

@csoneson
Copy link
Author

csoneson commented Feb 9, 2024

Thanks again - just let me know if there's something I can do!
I am also able build the package locally in less than 10 minutes, but of course there's not really anything else running on my laptop at the same time.

@vjcitn
Copy link
Collaborator

vjcitn commented Feb 9, 2024

Hi -- when you say "build the package locally in 10 min" does that include all the miniconda activities for a version bump?

@hpages
Copy link
Contributor

hpages commented Feb 9, 2024

when you say "build the package locally in 10 min" does that include all the miniconda activities for a version bump?

For me when I did it on merida1 it did include all the miniconda activities, yes.

Anyways, it doesn't seem fair to take into account the time it takes to set up the basilisk environment. This is just a one-time overhead and doesn't reflect the efficiency of the code in the package itself.

One way to ignore the overhead would be to have the SPB (and BBS) run R CMD build twice on the package. The first time would be behind the scene and with more time allowed, and only the second time would "count" and be reported. This would be done only for a small subset of packages that are manually enrolled via some flag that they add to their .BBSoptions file. However, this would require some substantial changes to the SPB and BBS code base (unfortunately the two build systems don't share as much code as they could). So this is something that we might consider doing in the future.

In the meantime, I'm going to manually restart a build. If it passes R CMD build on merida1 in time, hopefully we can give it the green light for ingestion.

@bioc-issue-bot
Copy link
Collaborator

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings
on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder:
Linux (Ubuntu 22.04.3 LTS): sketchR_0.99.2.tar.gz
macOS 12.7.1 Monterey: sketchR_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/sketchR to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.

@bioc-issue-bot bioc-issue-bot added OK and removed TIMEOUT labels Feb 9, 2024
@hpages
Copy link
Contributor

hpages commented Feb 9, 2024

Build and check each took about 2 min on nebbiolo1 and merida1. Yeah!

@csoneson
Copy link
Author

csoneson commented Feb 9, 2024

Hi -- when you say "build the package locally in 10 min" does that include all the miniconda activities for a version bump?

Yes, I think so (at least as far as I can tell), and it was rebuilding the environment (but may potentially have used some previously downloaded and cached packages rather than re-downloading them, I didn't actually check that in detail).

Build and check each took about 2 min on nebbiolo1 and merida1. Yeah!

Yay!!

@hpages
Copy link
Contributor

hpages commented Feb 9, 2024

Can we clear sketchR for ingestion @vjcitn @lshep?

@lshep
Copy link
Contributor

lshep commented Feb 12, 2024

A reviewer hasn't looked at the package yet. I can take a more in depth look this week.

@lshep lshep self-assigned this Feb 12, 2024
@lshep
Copy link
Contributor

lshep commented Feb 15, 2024

When I'm trying to build locally I'm getting the following ERROR is there something I should do to resolve?

lorikern@jbcj433:~/PkgReview$ R CMD build sketchR 
* checking for file 'sketchR/DESCRIPTION' ... OK
* preparing 'sketchR':
* checking DESCRIPTION meta-information ... OK
* installing the package to build vignettes
* creating vignettes ... ERROR
--- re-building ‘sketchR.Rmd’ using rmarkdown

Quitting from lines 60-89 [unnamed-chunk-2] (sketchR.Rmd)
Error: processing vignette 'sketchR.Rmd' failed with diagnostics:
function 'as_cholmod_sparse' not provided by package 'Matrix'
--- failed re-building ‘sketchR.Rmd’

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

Error: Vignette re-building failed.
Execution halted

@csoneson
Copy link
Author

csoneson commented Feb 15, 2024

I've seen this happening when some packages needed to be reinstalled (from source) since Matrix changed their ABI in v1.6-2. For me, reinstalling the packages mentioned in https://stat.ethz.ch/pipermail/r-package-devel/2023q4/010051.html (plus TFBSTools, but that's not used here) has been enough.

@lshep
Copy link
Contributor

lshep commented Feb 15, 2024

hmm... I just tried re-instaling Matrix from source but it still didnt work. Maybe I'll try re-installing R ...

@csoneson
Copy link
Author

I think reinstalling Matrix itself is not enough, also the packages linking to it must be reinstalled (the ones in the linked post).

@lshep
Copy link
Contributor

lshep commented Feb 15, 2024

tried both Matrix and TFBSTools and still same...

@csoneson
Copy link
Author

What about irlba and/or lme4?

@lshep
Copy link
Contributor

lshep commented Feb 15, 2024

I deleted out and re-installed Matrix, TFBSTools, lme4, and irlba .... and that seemed to work. I'll try to have a review by tomorrow

@lshep
Copy link
Contributor

lshep commented Feb 22, 2024

A few quick comments:
LICENSE

  • Is it meant to be 2022?

vignette

  • Can the abstract more distinctly describe the "main functionalities" of
    sketchR? Why its important to add to bioconductor/what is novel about the
    package.

man

  • Can you provide a package level man page so when a naive user tries
    ?sketchR it suggests functionality or where to start with the package.

@bioc-issue-bot
Copy link
Collaborator

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

@csoneson
Copy link
Author

Thanks! We've addressed the comments here: https://github.com/fmicompbio/sketchR/pull/13/files

Is it meant to be 2022?

Actually yes - from what we understand the copyright year should be when the software was first made available to the public, and in this case the package has been on GitHub since 2022.

Can the abstract more distinctly describe the "main functionalities" of sketchR?

We've expanded the introduction to cover these aspects.

Can you provide a package level man page?

Yep, good catch, I forgot that - added.

@bioc-issue-bot
Copy link
Collaborator

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings
on all platforms.

Please see the build report for more details.

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

@lshep lshep added 3a. accepted will be ingested into Bioconductor daily builder for distribution and removed pre-review on bioconductor git and access to on demand build but not assigned reviewer until build report clean labels Feb 26, 2024
@bioc-issue-bot
Copy link
Collaborator

Your package has been accepted. It will be added to the
Bioconductor nightly builds.

Thank you for contributing to Bioconductor!

Reviewers for Bioconductor packages are volunteers from the Bioconductor
community. If you are interested in becoming a Bioconductor package
reviewer, please see Reviewers Expectations.

@lshep
Copy link
Contributor

lshep commented Mar 4, 2024

The default branch of your GitHub repository has been added to Bioconductor's
git repository as branch devel.

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/csoneson.keys is not empty), then no further steps are required. Otherwise, do the following:

  1. Add an SSH key to your github account
  2. Submit your SSH key to Bioconductor

See further instructions at

https://bioconductor.org/developers/how-to/git/

for working with this repository. See especially

https://bioconductor.org/developers/how-to/git/new-package-workflow/
https://bioconductor.org/developers/how-to/git/sync-existing-repositories/

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using BiocManager::install("sketchR"). The package 'landing page' will be created at

https://bioconductor.org/packages/sketchR

If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.

@lshep lshep closed this as completed Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3a. accepted will be ingested into Bioconductor daily builder for distribution OK
Projects
None yet
Development

No branches or pull requests

5 participants