-
Notifications
You must be signed in to change notification settings - Fork 11
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
Adds a new example pipeline and edits existing example pipeline #53
Conversation
Codecov Report
@@ Coverage Diff @@
## master #53 +/- ##
=======================================
Coverage 80.64% 80.64%
=======================================
Files 36 36
Lines 806 806
Branches 99 99
=======================================
Hits 650 650
Misses 156 156 Continue to review full report at Codecov.
|
I think the tutorial should use
or maybe
|
const watermill = require('../../..') /* don't forget to edit this require if | ||
you run it outside bionode-watermill examples/pipelines/simple_tutorial | ||
folder */ | ||
const task = watermill.task /* have to specify task since bionode-watermill |
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.
task is not an operator
params: 'Test_file.txt', /*defines parameters to be passed to the | ||
task function*/ | ||
name: 'This is the task name' //defines the name of the task | ||
}, ({ params }) => `touch ${params}` |
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.
might be worthwhile to use
function (resolvedProps) {
const params = resolvedProps.params
// ...
}
before introducing ES6 syntax
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.
I think you have a point. ES6 is not standard js, so we should exemplify it first with standard js.
Then after defining the task, it may be executed like this: | ||
```javascript | ||
// runs the task | ||
simpleTask() |
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.
mention that it returns a promise, and also can take a callback
Oh OK i see you have another "how to use in your project" part. Hmm ok, I'm not sure if |
For now, i think the best pratice for the tutorial would be to require('bionode-watermill') instead of the require('../../..'). Therefore, to make it less confusing, for the user, maybe the best way is to create a new repo that the user just have to clone in order to make the tutorial. Because it might be confusing where to create the new directory, since if you make the new diretory inside the bionode-watermill folder it won't work. |
I have removed the tutorials and created a new repo for the tutorial. So, here I just left the new pipeline example a minor change to |
This PR adds a new example pipeline intended do document an easy to use tutorial with simple NGS tools. Also it adds some changes to the existing variant calling pipeline by adding changing the size of the download size of reads to a smaller one, reducing the time to download reads as well as the download size itself.