Bioconductor / Contributions

Contribute Packages to Bioconductor
131 stars 33 forks source link

funomics package #3322

Closed elisagdelope closed 3 weeks ago

elisagdelope commented 4 months 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 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 commented 4 months ago

Hi @elisagdelope

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: funomics
Title: Aggregating Omics Data into Higher-Level Functional Representations
Version: 0.99.0
Authors@R: c(
 person("Elisa", "Gomez de Lope", role = c("aut", "cre"), email = "elisa.gomezdelope@uni.lu",comment=c(ORCID="0000-0002-7115-7393")),
 person("Enrico", "Glaab", role = "ctb", email = "enrico.glaab@uni.lu", comment = c(ORCID="0000-0003-3977-7469")))
Description: The 'funomics' package ggregates or summarizes omics data into 
  higher level functional representations such as GO terms gene sets 
  or KEGG metabolic pathways. The aggregated data matrix represents
  functional activity scores that facilitate the analysis of 
  functional molecular sets while allowing to reduce dimensionality 
  and provide easier and faster biological interpretations. 
  Coordinated functional activity scores can be as informative as 
  single molecules!
License: MIT + file LICENSE
Encoding: UTF-8
RoxygenNote: 7.3.1
URL: https://github.com/elisagdelope/funomics
BugReports: https://github.com/elisagdelope/funomics/issues
Depends:
    R (>= 4.3.0)
Imports: NMF,  pathifier, stats
Suggests:
    knitr,
    rmarkdown,
    testthat (>= 3.0.0)
VignetteBuilder: knitr
VignetteIndexEntry: funomics_vignette Vignette
biocViews: Software, Transcriptomics, Metabolomics, Proteomics, Pathways, GO, KEGG
Config/testthat/edition: 3
lshep commented 3 months ago

Please make sure R CMD build, R CMD check, and BiocCheck can run on your package without ERROR. Your package currently can not be built as it is missing a NAMESPACE file.

Could you please provide an abstract/intro section in your vignette that provides motivation for inclusion in Bioconductor and when appropriate a review and comparison to existing Bioconductor packages with similar functionality or scope. It will also be important to show interoperability with some of the standard Bioconductor classes for pathway analysis and gene sets like KEGGREST, Gostats, BiocSet/GSEAset, etc. See also common classes or results of pathway biocview

elisagdelope commented 3 months ago

Thanks for the revision. The NAMESPACE was not in the repository because it was mistakenly included in the .gitignore file, I already fixed it. The R CMD build, check and BiocCheck are passed smoothly. I have updated the intro section in the vignette with a motivation and comparison with existing Bioconductor packages.

As for the interoperability, the inputs for the package are essentially an omics data matrix or dataframe and the molecular sets in the format of a list of lists. This list of lists is a generic format, not tied to any particular package or class and can be obtained by processing the downloads from https://data.broadinstitute.org/gsea-msigdb/msigdb/release/ or https://mips.helmholtz-muenchen.de/corum/#download/ as described in the vignette section "Real data: Where to find molecular sets?". To better clarify, I added to the vignette both an example of how to read the downloads from those sources to get a list of lists containing pathways and molecules (e.g. genes), and another code snippet for how to retrieve pathway gene sets as a list of lists from KEGGREST, but the idea is that the user uses their preferred source of molecular sets as desired for their analyses and these are just some examples.

Regarding GOstats or GSEAset, these packages do not align with the current scenario because they are meant to test enrichment and thus require a "universe of genes". Please note that enrichment is not the purpose of funomics package.

All changes have been uploaded to the repository linked in this issue.

vjcitn commented 3 months ago

Thanks for this submission. I understand that the package code handles data matrices or data frames and lists of lists. So we have, in the vignette,

X <- matrix(abs(rnorm(g * s)), nrow = g, dimnames = list(paste0("g", 1:g), paste0("s", 1:s)))
pathways <- as.list(sample(10:100, size = 100, replace = TRUE))
pathways <- lapply(pathways, function(s, g) paste0("g", sample(1:g, size = s, replace = FALSE)), g)
names(pathways) <- paste0("pathway", seq_along(pathways))
print(paste("Dimensions of omics matrix X:", dim(X)[1], "*", dim(X)[2], "; Length of molecular sets list:", length(pathways)))

The aim of Bioconductor data structure designs is to hide the complexity of code like this. The omics data matrix should be part of a SummarizedExperiment in which omic features and sample characteristics are well-annotated and bound together. Methods on SummarizedExperiments support high-level expression of filtering on features or sample characteristics. If you don't wish to use SummarizedExperiments or interoperate with other Bioconductor components, please explain further in this issue.

elisagdelope commented 3 months ago

Thanks a lot for your revision and comment. This code from the vignette corresponds to the generation of simulated data: A matrix X of omics abundance (e.g., gene counts) is generated, as well as a list of lists containing the genes involved in pathways. I think this code would not be less complex if using a SummarizedExperiment object instead of a matrix and a list of lists. The main value of SummarizedExperiment is to have sample annotations and the counts matrix in the same object, as you mention indeed. However, in the context of this package, the pathway information is independent of sample-specific annotations typically stored in the colData slot of a SummarizedExperiment object (e.g., treatment, diagnosis). The focus in the vignette is on demonstrating the core functionalities of funomics for pathway-level aggregation, and sample annotations are not essential for this purpose.

More importantly, the pathway list of lists contains information about the molecular sets and their constituent molecules (e.g., gene IDs), and is typically downloaded or obtained from external sources (as described in the vignette, there is a section on this, and how to integrate programatically with KEGGREST), hence a separate list aligns well with how pathway information is often obtained in practice, beyond the simulated pathway list in the vignette.

Therefore, a data matrix/dataframe and a separate list of molecular sets provides flexibility to work with various data formats, such as for users whose data does not necessarily come from a SummarizedExperiment object. Note that if the actual data is in SummarizedExperiment format, the assay matrix can easily be retrieved (assay(SEobject)) to be used as input for funomics. I added a comment on this in the vignette to cover this case.

I've also updated the repository with a slight change in the package name (funomics -> funOmics). Please, let me know how i should proceed.

vjcitn commented 3 months ago

You are correct on all counts. But the contribution guidelines are clear: Interoperate with other Bioconductor packages by re-using common data structures (see Common Bioconductor Methods and Classes) and existing infrastructure (e.g., rtracklayer::import() for input of common genomic files). I get that you are reluctant to do this, and you explain that users who already have SummarizedExperiment or gene set data structures can disassemble them to use your package. The "ecosystem" and "interoperability" concepts are somewhat vague, but wouldn't your package be appropriate for CRAN? We have limited review resources and since you are not going to use Bioconductor data structures to any advantage, it would be better if our reviewers could pass on this one. Close the issue if you agree. Thanks again.

elisagdelope commented 3 months ago

Thank you for your feedback. I understand and appreciate the emphasis on Bioconductor contribution guidelines. While I initially considered CRAN due to the package's flexibility in handling diverse data formats, I've been advised and encouraged that 'funomics' core functionality of aggregating omics data into pathway-level features aligns better with Bioconductor's focus.

The package funomics interoperates with the Bioconductor ecosystem in several ways. It leverages KEGGREST for retrieving pathway information, a common Bioconductor package for accessing KEGG resources. Additionally, funomics potentially complements packages like pathifier by offering alternative functionalities for pathway-level aggregation (e.g., user-defined pooling operators). Indeed, funomics package provides a kind of wrapper for the pathifier function "quantify_pathways_deregulation". I've also realized pathifier package performs somewhat similar operations, and does not require input from SummarizedExperiments object, hence I guess it is not a strict requirement?

Nevertheless I understand the benefits of SummarizedExperiment for data management. Disassembling and reassembling SummarizedExperiment objects within funomics could be technically feasible, and I'm not reluctant to work on this, but it introduces unnecessary complexity (at this stage, this would artificially force the package to use it without real necessity). However, I'm open to exploring ways to integrate SummarizedExperiment or other Bioconductor structures in future versions of the package based on specific use cases and user feedback where using it provides added value to their analyses (this is why I point to the use of github issues to suggest improvements).

Given funomics' focus on functional-level omics data aggregation and its interoperability with Bioconductor packages, I believe it can still be a valuable addition to the bioconductor repository and community even if at this stage, it does not take SummarizedExperiments objects as input. It operates with other packages, demonstrates utility in the field, and can also interact with the assay slot of SummarizedExperiments objects, so it's compatible with bioconductor structures.

I'm happy to discuss this further and address any additional questions you might have. If absolutely required, as mentioned I can artifically force funomics to take SummarizedExperiment object as input, disassembling it internally, and assembling a SummarizedExperiment as an output, but I believe doing this just for the sake of using this class is not necessarily a good practice.

vjcitn commented 3 months ago

Thank you. The only "necessity" I am thinking of at this point is fairness. Many developers of packages in the ecosystem took the time to achieve interoperability and did not get reviewed until this was accomplished. I think your package has to meet the same standard. Please revise.

elisagdelope commented 3 months ago

Hi, I'm not sure I follow your last comment. I pointed some examples in which funomics package interacts with other packages of the bioconductor ecosystem, hence the intereoperability. I also pointed an example bioconductor package using omics data where SummarizedExperiment in particular is not used. Most importantly, I explained why using SummarizedExperiment input does not add much value at this stage, indeed if only SummarizedExperiment structures are accepted as input, it would limit its flexibility and restrict its use to this specific data format, which is not necessarily the data format all potential users of funomics implement.

Then, I don't really understand what is the requirement that is missing for the package to move forward. Are there alternative ways funomics can demonstrate interoperability with the Bioconductor ecosystem beyond SummarizedExperiment integration? or can you elaborate on how this SummarizedExperiment integration could make sense given the functionality and current structure of the package?

lshep commented 3 months ago

I would argue then show an example using one of the many available SummarizedExperiment objects rather than a toss away comment in the vignette as a potential extension of the "real work" example in the vignette. There are many predefined SummarizedExperiments provided in packages and in the ExpeirmentHub.

Also, the work around I generally suggest for people that have extensively coded out generalized structures rather than Bioconductor structures, if their documentation and vignette are thorough and code fairly sound and they argue its not as much value, is to create wrapper function around Bioconductor objects therefore simply adding a secondary function specific for the Bioconductor class structure; or simply check for the input to a function and if its a SummarizedExperiment (inherits() or is()) parse out what is needed by your functions internally. Is there a way to do this so both are implemented?

Similarly you use KEGGREST (which is good) but then there is a complicated looping section to force it into your structure.

kegg_sets <- list()
for (i in seq(1, length(pathway_ids), by = batch_size)) {
  batch_ids <- pathway_ids[i:(min(i + batch_size - 1, length(pathway_ids)))]
  batch_objs <- keggGet(batch_ids)
  for (obj in batch_objs) {
    if ("GENE" %in% names(obj)) {
      entrez_id_indices <- seq(1, length(obj$GENE), by = 2) # Extract Entrez IDs (odd entries)
      entrez_ids <- obj$GENE[entrez_id_indices]
      kegg_sets[[obj$PATHWAY_MAP[[1]]]] <- entrez_ids
}}}

As Vince said above, this is the complexity we like to try to minimize for users. Where we would suggest again, a function that would take care of this formatting for a user.

We will move this into the next stage of review but would expect some additional work to make it easier for end users.

lshep commented 3 months ago

As far as the renaming, the DESCRIPTION and the repo must match exactly including capitalization. Please also rename the repo and update the url in the first comment.

bioc-issue-bot commented 3 months ago

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 commented 3 months ago

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: "ABNORMAL". 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: ERROR before build products produced.

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/funomics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

lshep commented 3 months ago

I think this has to do with the change of capitalization. I thought it would work to just update. I'll look into it more this weekend and get back to you on if we can change it on our end or what steps to take next.

lshep commented 3 months ago

I think everything is straightened out. You should receive a new report shortly

bioc-issue-bot commented 3 months ago

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

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

elisagdelope commented 3 months ago

Thanks both for your comments and understanding:

Since there is this "abnormality", just to avoid messing the bioconductor git I will keep updating my personal repo ( https://github.com/elisagdelope/funOmics) until you let me know about next steps.

bioc-issue-bot commented 3 months ago

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

bioc-issue-bot commented 3 months ago

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

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

bioc-issue-bot commented 3 months ago

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

bioc-issue-bot commented 3 months ago

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 22.04.3 LTS): funOmics_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 git@git.bioconductor.org:packages/funOmics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 months ago

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

bioc-issue-bot commented 3 months ago

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 22.04.3 LTS): funOmics_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 git@git.bioconductor.org:packages/funOmics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 months ago

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

bioc-issue-bot commented 3 months ago

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): funOmics_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 git@git.bioconductor.org:packages/funOmics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 months ago

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 commented 3 months ago

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

bioc-issue-bot commented 3 months ago

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): funOmics_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 git@git.bioconductor.org:packages/funOmics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

elisagdelope commented 3 months ago

Thanks both for your comments and understanding:

  • I will show an example using one SummarizedExperiment object from ExpeirmentHub in the vignette.
  • I will wrap the code for getting kegg pathways from KEGGREST in a list of lists format into a function.
  • I will revise if/how to better integrate a SummarziedExperiment as input (and then I guess output as well). - this might take some more time tho.

Since there is this "abnormality", just to avoid messing the bioconductor git I will keep updating my personal repo ( https://github.com/elisagdelope/funOmics) until you let me know about next steps.

An example using the SummarizedExperiment object from airway data and a new function get_kegg_sets() to retrieve pathway gene sets from KEGG have been implemented. Since SummarizedExperiment assays must be of the same dimensions, and funOmics reduces the dimensions (in particular, the number of features) of the omics matrix, the vignette extends the example usage for the airway SummarizedExperiment object to interact with another bioconductor package, MultiAssayExperiment, that provides a structure similar to SummarizedExperiment but allowing to include the information in airway as well as the summarized data matrix provided by funOmics.

I think the vignette is now more complete, and shows effectively how funOmics can be integrated in different workflows and analyses, while still being flexible for various data formats.

Re the "abnormal" label, it seems like it still assigns it when I push.

lshep commented 3 months ago

I'll check why the abnormal tag is still being added but it looks like it was resolved since you pushed and received a build report so please continue to do so.

elisagdelope commented 3 months ago

Thanks, from the report I see this note "License stub is invalid DCF." and I don't really understand what is to be done here, as I do have the MIT license file in the repository, and it is indicated in the DESCRIPTION file.

Regarding the 'suppressWarnings'/'*Messages' if possible (found 2 times), I have used it on suppressMessages(AnnotationDbi::mapIds function()) that is used internally in get_kegg_sets() function to avoid displaying messages coming from a gene id internal mapping that could confound the user (as those messages are not related to the core purpose of get_kegg_sets(), and are not always shown, only if using homo sapiens samples). I think having them displayed is quite unclear in the context of get_kegg_sets, hence the suppressMessages.

Please let me know about these, and if there's anything else that needs to be addressed.

elisagdelope commented 2 months ago

Hi, I addressed the concerns on interoperability and added a function to get KEGG sets as suggested. The package build without warnings/errors. Are there any updates on the package revision?

vjcitn commented 2 months ago

There will be. Feel free to make further improvements.

vjcitn commented 2 months ago

Early in the vignette we have

# Example usage:
set.seed(1)
g <- 10000
s <- 20
X <- matrix(abs(rnorm(g * s)), nrow = g, dimnames = list(paste0("g", 1:g), paste0("s", 1:s)))
pathways <- as.list(sample(10:100, size = 100, replace = TRUE))
pathways <- lapply(pathways, function(s, g) paste0("g", sample(1:g, size = s, replace = FALSE)), g)
names(pathways) <- paste0("pathway", seq_along(pathways))
print(paste("Dimensions of omics matrix X:", dim(X)[1], "*", dim(X)[2], "; Length of molecular sets list:", length(pathways)))

I commented previously that code like this in a vignette is unwelcome. Later in the vignette you illustrate with airway dataset. This should be moved up as it is potentially biologically meaningful; the example with simulation is not.

The vignette mentions

funOmics accommodates various omics modalities (e.g., metabolomics, transcriptomics, proteomics), and allows users to define custom molecular sets for aggregation.

Are any of these tasks illustrated in the package or in literature? Please clarify.

vjcitn commented 2 months ago

It seems to me that there are some omissions in the package that the package checking system is missing. There should be declarations of usage of airway, SummarizedExperiment, and MultiAssayExperiment in DESCRIPTION. Probably in Suggests field. Set the environment variable _R_CHECK_SUGGESTS_ONLY_ to a true value to see that the absence of these declarations causes CHECK to fail.

lshep commented 2 months ago

I do think we are getting closer to acceptance. We appreciate your efforts thus far. A few additional comments:

man

vignette

print(paste("Pathway activity score matrix has dimensions:", nrow(pathway_activity), ",", ncol(pathway_activity)))

and your summary statistics

length_sets <- sapply(pathways, function(p) length(p))
short_sets <- names(length_sets[length_sets < min])
print(length_sets[short_sets])
print(pathways[short_sets])

this is where defining a class structure and having dedicated helper functions that would do the subsetting and have nice print statements could benefit the user. Remembering specific code subsets that need to be performed in order to extract information from your list of lists can be complex and cumbersome for an end user.

bioc-issue-bot commented 1 month ago

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

elisagdelope commented 1 month ago

Pending restructure of the vignette to highlight the use case on airway data as compared to simulated data.

bioc-issue-bot commented 1 month ago

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): funOmics_0.99.6.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 git@git.bioconductor.org:packages/funOmics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

lshep commented 1 month ago

Thank you. Let us know when the vignette is updated and we will take a look again.

bioc-issue-bot commented 1 month ago

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

bioc-issue-bot commented 1 month ago

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: ERROR before build products produced.

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/funOmics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

elisagdelope commented 1 month ago

I'm currently facing an issue with the NMF package. One of the functions in funOmics uses this package. The package is imported both in the NAMESPACE file and in the functions.R file, where the functions are located (in roxygen2 format @import, @importFrom). I have also tried to call the functions as NMF::nmf() and NMF::coef() instead of just nmf() and coef(), however the error persists.

The error (see below) only gets removed if I explicitly load the library itself, like, if I have library(NMF) before calling the package function that, internally, uses one of its functions. I have explored a range of possible solutions but i have not been successful. What would you recommend? I know it's not ideal but, is it fine if i add library(NMF) in the vignette? Weirdly enough, this issue only happens with this library, not with any other.

Error traceback:

Error in `do.call()`:
! 'what' must be a function or character string
Backtrace:
  1. funOmics::summarize_pathway_level(X, kegg_sets, type = "nmf")
  2. funOmics:::aggby_dimred(ifunmat, type)
  4. NMF::nmf(X, rank = 1)
  6. NMF::nmf(x, rank, NULL, ...)
  7. NMF (local) .local(x, rank, method, ...)
  9. NMF::nmf(x, rank, method, seed = seed, model = model, ...)
 11. NMF::nmf(x, rank, method = strategy, ...)
 12. NMF (local) .local(x, rank, method, ...)
 13. base::do.call(...)

When in vignette:

|...........................                        |  54% [unnamed-chunk-18]
Quitting from lines  at lines 182-184 [unnamed-chunk-18] (funomics_vignette.Rmd)

Error in `do.call()`:
! 'what' must be a function or character string
Backtrace:
  1. funOmics::summarize_pathway_level(X, kegg_sets, type = "nmf")
  2. funOmics:::aggby_dimred(ifunmat, type)
  4. NMF::nmf(X, rank = 1)
  6. NMF::nmf(x, rank, NULL, ...)
  7. NMF (local) .local(x, rank, method, ...)
  9. NMF::nmf(x, rank, method, seed = seed, model = model, ...)
 11. NMF::nmf(x, rank, method = strategy, ...)
 12. NMF (local) .local(x, rank, method, ...)
 13. base::do.call(...)
Execution halted
vjcitn commented 1 month ago

feel free to post this at the developer forum on slack

bioc-issue-bot commented 1 month ago

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

bioc-issue-bot commented 1 month ago

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): funOmics_0.99.8.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 git@git.bioconductor.org:packages/funOmics to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

elisagdelope commented 1 month ago

I think now the vignette is quite illustrative and straightforward to follow. Looking forward to your review.

bioc-issue-bot commented 3 weeks ago

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 commented 3 weeks ago

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/elisagdelope.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("funOmics"). The package 'landing page' will be created at

https://bioconductor.org/packages/funOmics

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.