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

Patch: bugs in sleuth_results & other miscellaneous fixes #163

Merged
merged 8 commits into from
Apr 17, 2018

Conversation

warrenmcg
Copy link
Collaborator

@warrenmcg warrenmcg commented Mar 21, 2018

Hi @pimentel,

sleuth_results bugs:

I discovered two bugs in sleuth_results:

  1. setting show_all = F still results in all target_ids being returned.
  2. setting pvalue_aggregate = T results in the extra annotation data being missing.

I made the following changes to the function to hopefully correct this behavior.

spread_abundance_by speed-up

I also earlier had made a faster version of spread_abundance_by, and realized that it make it in the propagation forward of merged changes. That version is found below, and I have confirmed that it produces identical results in at least three separate datasets with different target_ids. Yet, it has a 3-fold increase in speed. It's a small gain overall, but it greatly affects the sleuth_live experience since plotting functions heavily use spread_abundance_by, and it affects to a small extent sleuth-ALR overall speed.

NAMESPACE issue with transform_status

When running devtools::document(), the NAMESPACE updated transform_status from a regular method to an S3method.

sample_to_covariates data.table error, addressing issue #153

When a sample_to_covariates table is provided to sleuth_prep as a data.table object, it passes the data frame check in place, but causes downstream problems. I put in a line to coerce any table that passes the check to a data frame.

Best,
Warren

@warrenmcg warrenmcg changed the title Patch: show_all bug in sleuth_results Patch: show_all bug in sleuth_results & spread_abundance_by speedup Mar 23, 2018
@warrenmcg warrenmcg changed the title Patch: show_all bug in sleuth_results & spread_abundance_by speedup Patch: bugs in sleuth_results & spread_abundance_by speedup Mar 23, 2018
@warrenmcg warrenmcg changed the title Patch: bugs in sleuth_results & spread_abundance_by speedup Patch: bugs in sleuth_results & other miscellaneous fixes Mar 26, 2018
@warrenmcg warrenmcg requested a review from pimentel March 26, 2018 14:41
+ also added a warning if metadata for the gene-level analyses results in multiple entries for any genes.
+ show_all before pval_aggregate broke the lancaster function, so it's now after
+ show_all and combining metadata works the same for both pval_aggregate and gene_mode,
but only if pval_aggregate maintains that the first column is called "target_id"
+ read_kallisto now always records the number of bootstraps present and the number that will be used for analysis,
regardless of whether it actually reads those bootstraps
+ the number of bootstraps found and used are now stored as attributes for the kallisto object
+ the summary function for sleuth objects will now print both the number of bootstraps found and the number used for analysis
@pimentel
Copy link
Collaborator

Looks great -- as usual, thanks, @warrenmcg!

@pimentel pimentel merged commit 359d29f into pachterlab:devel Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants