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

funomics package #3322

Open
10 tasks done
elisagdelope opened this issue Feb 26, 2024 · 47 comments
Open
10 tasks done

funomics package #3322

elisagdelope opened this issue Feb 26, 2024 · 47 comments
Assignees
Labels
2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place 3d. needs interop Package must explicitly use Bioconductor structures and methods ABNORMAL OK

Comments

@elisagdelope
Copy link

elisagdelope commented Feb 26, 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 @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

@bioc-issue-bot bioc-issue-bot added the 1. awaiting moderation submitted and waiting clearance to access resources label Feb 26, 2024
@lshep
Copy link
Contributor

lshep commented Mar 4, 2024

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

@lshep lshep added 3e. pending pre-review changes review has indicated blocking concern that needs attention 3d. needs interop Package must explicitly use Bioconductor structures and methods labels Mar 4, 2024
@elisagdelope
Copy link
Author

elisagdelope commented Mar 4, 2024

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
Copy link
Collaborator

vjcitn commented Mar 20, 2024

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
Copy link
Author

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
Copy link
Collaborator

vjcitn commented Mar 21, 2024

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
Copy link
Author

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
Copy link
Collaborator

vjcitn commented Mar 21, 2024

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
Copy link
Author

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
Copy link
Contributor

lshep commented Mar 22, 2024

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
Copy link
Contributor

lshep commented Mar 22, 2024

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.

@lshep lshep added pre-check passed pre-review performed and ready to be added to git and removed 3e. pending pre-review changes review has indicated blocking concern that needs attention labels Mar 22, 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 3d. needs interop Package must explicitly use Bioconductor structures and methods labels Mar 22, 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: "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
Copy link
Contributor

lshep commented Mar 22, 2024

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 lshep added the 3d. needs interop Package must explicitly use Bioconductor structures and methods label Mar 22, 2024
@lshep
Copy link
Contributor

lshep commented Mar 22, 2024

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

@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: "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
Copy link
Author

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.

@bioc-issue-bot
Copy link
Collaborator

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

@lshep lshep added the 2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place label Mar 25, 2024
@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Collaborator

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

@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): 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
Copy link
Author

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
Copy link
Contributor

lshep commented Mar 25, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

vjcitn commented Apr 9, 2024

There will be. Feel free to make further improvements.

@vjcitn
Copy link
Collaborator

vjcitn commented Apr 15, 2024

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
Copy link
Collaborator

vjcitn commented Apr 15, 2024

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
Copy link
Contributor

lshep commented Apr 16, 2024

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

man

  • Please have a package level man page so if a naive user does ?funOmics
    something is displayed

vignette

  • I would agree with @vjcitn. Switching to have the real world data more prevalent
    over simulated data would be preferred. It should at least be above the
    sessionInfo at minimum. Thank you for adding the real world data set with
    airways to better show the interaction.

  • So to clarify on @vjcitn statement a little. You have code displaying useful
    information. Things like your print statements

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
Copy link
Collaborator

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

@elisagdelope
Copy link
Author

  • A package level man page for ?funOmics has been added.
  • Packages needed for the vignette (but not dependencies of the package) have been added to the suggest of the description file.

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

@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): 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
Copy link
Contributor

lshep commented May 9, 2024

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

@bioc-issue-bot
Copy link
Collaborator

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

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

@bioc-issue-bot bioc-issue-bot added ERROR and removed OK labels May 14, 2024
@elisagdelope
Copy link
Author

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
Copy link
Collaborator

vjcitn commented May 15, 2024

feel free to post this at the developer forum on slack

@bioc-issue-bot
Copy link
Collaborator

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

@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): 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.

@bioc-issue-bot bioc-issue-bot added OK and removed ERROR labels May 15, 2024
@elisagdelope
Copy link
Author

elisagdelope commented May 15, 2024

  • I have restructured the vignette, now almost everything is illustrated through the airway dataset (summarizedExperiment). I have also added examples from all types of aggregations available in funOmics (dimension reduction, summary statistics and test statistics).
  • I have added a new function, short_sets_details so that some interesting information about molecular sets below an specified minsize (minimum size) that I was previously showing through manual inspection of results is now accessible through this helper function without need to extract that information manually from the list of lists.
  • The relevant print statements have been incorporated in the package functions so this info is automatically shown without need for further printing, again removing need for manual inspection from the user.
  • Re the NMF package issue: I was suggested in the developer forum (slack) to go with the Depends (it appears there is a do.call() somewhere in NMF package that assumes the object is on the search path), and in the meantime i've opened an issue on NMF to see if this can be perhaps solved in their end.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place 3d. needs interop Package must explicitly use Bioconductor structures and methods ABNORMAL OK
Projects
None yet
Development

No branches or pull requests

4 participants