search

cgat-developers / cgat-flow

cgat-flow repository
MIT License
13 stars 9 forks source link

Add peakcalling memory options to pipeline.yml #72

Closed kevinrue closed 5 years ago

kevinrue commented 5 years ago

I left a bunch of print statements related to debugging. I'll remove them if I get positive feedback about merging this PR to master

kevinrue commented 5 years ago

One of the debug print statements was causing trouble because design.tsv is not in the working directory during the nosetests, I removed all of the print statements. If this PR is considered for merge, the default memorysettings in pipeline.yml shoud just be given some extra thought to work with most systems.

Acribbs commented 5 years ago

Have you managed to run the pipeline testing pipeline to see if this breaks out tests? If not then I will wait until I can find some time to test.

Acribbs commented 5 years ago

Actually, it most likely will break tests because of modifications to the pipeline ini. I will have a look when I can and get back to you.

kevinrue commented 5 years ago

Well the pipeline hasn't broken at runtime for me, because I have design.tsv and the input files in the right places. Which is why I forgot it would break during the tests, because those aren't setting up an actual project I suppose.For a split second, I saw a successful build on Travis, but I guess you restarted it, as it's running again (?)

Acribbs commented 5 years ago

The pipeline testing framework may break though because your changes won't be present in the test yml file. I can look into this. Yes I restarted it as it times out for linux but passed for mac, it sometimes sticks because of conda. Hopefully restarting it will be resolve it.

Acribbs commented 5 years ago

Again, will run tests and merge if they pass

Acribbs commented 5 years ago

@kevinrue I cannot see this branch in git?

kevinrue commented 5 years ago

I only pushed it to my repository. https://github.com/kevinrue/cgat-flow/tree/KR_jobmemory_peakcalling

I can push it here as well if it makes a difference

kevinrue commented 5 years ago

I've pushed the same branch to this repo. https://github.com/cgat-developers/cgat-flow/tree/KR_jobmemory_peakcalling

Acribbs commented 5 years ago

If you think it makes sense and is going to be useful for others. I will perform pipeline testing if you do.

Acribbs commented 5 years ago

I've pushed the same branch to this repo. https://github.com/cgat-developers/cgat-flow/tree/KR_jobmemory_peakcalling

Ah I have just seen this.

kevinrue commented 5 years ago

Well. Those edits are necessary for me to get through the filterBams task for certain samples. On that basis, I would argue that it should be useful to others facing the same issue as I do. That said, @Charlie-George agreed to revisit the task at some point, to avoid the issue altogether. Even so, I don't think those edits would hurt: they are merely intended to allow users to control existing values that already have a reasonable default.

Acribbs commented 5 years ago

These tests now pass for me, maybe @Charlie-George wants to have a look over before merging