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

Issue with create_background - found in EWCE #22

Closed
Al-Murphy opened this issue Aug 16, 2022 · 4 comments
Closed

Issue with create_background - found in EWCE #22

Al-Murphy opened this issue Aug 16, 2022 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@Al-Murphy
Copy link
Contributor

Al-Murphy commented Aug 16, 2022

1. Bug description

create_background when all species are the same as the output species and background is not null. In this case currently the background input is ignored instead of just being returned:

species_list <- c(species1,species2)
    gene_var <- if(as_output_species) "ortholog_gene" else "input_gene"
    if(all(species_list==output_species)){
        #### If all species are the same, just use all_genes ####
        gene_map <- all_genes(species = output_species, 
                              method = method,
                              verbose = verbose)
        bg <- gene_map$Gene.Symbol
        return(bg)
    }

This should be changed to:

species_list <- c(species1,species2)
    gene_var <- if(as_output_species) "ortholog_gene" else "input_gene"
    if(all(species_list==output_species)){
        if(is.null(bg)){
            #### If all species are the same, just use all_genes ####
            gene_map <- all_genes(species = output_species, 
                                  method = method,
                                  verbose = verbose)
            bg <- gene_map$Gene.Symbol
        } else {
            bg <- unique(bg) 
        }
        return(bg)
    }

This affects EWCE in prepare_genesize_control_network and bootstrap_enrichment_test as instead of using the user's inputted background list, it is now creating a new one.

I've added code in EWCE to avoid this for now but it would be better to just update at orthogene

@bschilder
Copy link
Collaborator

Great catch, just added this. Thanks @Al-Murphy!

Will push as soon as i get past some annoying documentation issues that popped up:

> devtools::document()
ℹ Updating orthogene documentation
ℹ Loading orthogene
 Error: 'gprofiler_orgs' is not an exported object from 'namespace:orthogene'

@bschilder
Copy link
Collaborator

bschilder commented Aug 25, 2022

@Al-Murphy I noticed in MSS you both store sumstatsColHeaders as internal data and as a .rda file (essentially storing it in multiple locations, internal and external). Is there a reason you need to do both? My understanding was that you only have to do one or the other.

#' usethis::use_data(sumstatsColHeaders,overwrite = TRUE, internal=TRUE)
#' save(sumstatsColHeaders,
#'       file="data/sumstatsColHeaders.rda")

https://github.com/neurogenomics/MungeSumstats/blob/118e7f4cd94aa7b4c3af1b592aa811587200582a/R/data.R#L37

@Al-Murphy
Copy link
Contributor Author

Ah took me a while to remember, I store as an rda so users can more easily access it since it's meant to be editable for them! But yeah no reason other than that to do both (of course they could access it the other way but I just tried to make it as easy as possible)

@bschilder
Copy link
Collaborator

bschilder commented Aug 25, 2022

Ok, gotcha!

It turns out my issue was that in the data.R file I was specifying the name of the variable instead of NULL. This meant that devtools expected an exported object of that name. I also had to manually add the @name tag, as well as the @usage tag which isn't applicable to internal objects apparently:

before

....
#' @usage orthogene:::gprofiler_orgs
"gprofiler_orgs"

after

.... 
#' @name gprofiler_orgs
NULL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants