-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: main
Are you sure you want to change the base?
Dev caleb #10
Conversation
Catching up to main
… default, not a flag just in the if (TRUE) block
There was a problem hiding this 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?
@letaylor good idea-- added into the above commit. |
@henryjtaylor as soon as you give this the OK, I'll merge |
There was a problem hiding this 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"), |
There was a problem hiding this comment.
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'" |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
@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:
cp10k_filter
with flags forfilter_type
andfilter_threshold
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)nextflow.preview.dsl = 2
tonextflow.enable.dsl=2
due to deprication