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

Dev caleb #10

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Dev caleb #10

wants to merge 8 commits into from

Conversation

grenkoca
Copy link
Contributor

@henryjtaylor @letaylor

This is my custom branch I was using for the T2D KO work. Here are the changes I made, feel free to keep/discard what you want:

  1. Replaced config's cp10k_filter with flags for filter_type and filter_threshold
  2. Reduced 5 minute sleep to 15 second sleep.
  3. Added a bit of code to allow for new dispersion estimate fitting if the output range is too small. It is the bit in the else statement of the if (TRUE) block. I was going to remove it after I worked out the real issue, but figured it might be useful if we ever need to run DGE on something with a small range in input values. (i.e. fixes this issue)
  4. switched nextflow.preview.dsl = 2 to nextflow.enable.dsl=2 due to deprication

Copy link
Contributor

@letaylor letaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grenkoca one small comment and requested change.

After that, I think we should merge. @henryjtaylor what do you think?

bin/011-run_differential_expression.R Show resolved Hide resolved
@grenkoca
Copy link
Contributor Author

@letaylor good idea-- added into the above commit.

@grenkoca
Copy link
Contributor Author

@henryjtaylor as soon as you give this the OK, I'll merge

Copy link
Contributor

@henryjtaylor henryjtaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grenkoca This looks great! Could you make those quick changes I flagged?

The other quick thing, since this is a public repo, could you squash your commits to a single one, and just add a quick descriptor in the commit message?

help = "Filter to remove genes with fewer cp10k/counts
averages. See option `--filter_type`"
),
optparse::make_option(c("-x", "--filter_type"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do 'get_filter_phenotype'? Just to make it a bit more verbose

optparse::make_option(c("-x", "--filter_type"),
type = "character",
default = 1,
help = "Either 'counts' or 'cp10k'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you go ahead and add logcp10?

main.nf Outdated
@@ -1,6 +1,6 @@
#!/usr/bin/env nextflow

nextflow.preview.dsl = 2
nextflow.enable.dsl=2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when did they add this one? We're currently using v20.10.0.5430 -- if it's beyond that, we should add something to the README

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, but I'm on 22.04.3 and getting this error:

N E X T F L O W  ~  version 22.04.3
Preview nextflow mode 'preview' is not supported anymore -- Please use `nextflow.enable.dsl=2` instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: I found this blog post from July 2020 stating that nextflow.enable.dsl is available in v20.07.1, but also:

Note that the previous nextflow.preview directive is still available, however, when using the above declaration the use of the final syntax is enforced.

It's quitting the pipeline on later versions of Nextflow. See below for a fix to check the version and use the appropriate command

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.

3 participants