Closed antoniojbt closed 6 years ago
PS @sebastian-luna-valero Several commits passed tests but would have easily broken CGATPipelines. I have no suggestions at the moment as I didn't look in detail. Is any one pipeline executed as part of the tests? I'll check tests when possible though to get a better understanding.
Many thanks @AntonioJBT
The tests on travis do not run pipelines since the resources over there are quite limited (only a tiny virtual machine). However, we do so internally in our cluster with Jenkins but the testing results are not publicly accessible yet (only people with CGAT accounts can see it if they want to). I am planning to open it up at some point anyway.
I have rerun the Jenkins tests for this branch. To make it work internally I had to create a $HOME/.cgat
with:
[cluster]
queue=all.q
and all the tests passed.
Having said that, rather than merging these changes into the master now, I would wait for the CGAT core code to come out, since all pipelines would need refactoring anyway after we move in that direction.
@sebastian-luna-valero suggestions is sensible. Great stuff, @AntonioJBT . Having pipelines pip-installable sounds useful.
Hi @sebastian-luna-valero and @AndreasHeger, Thanks for testing, happy to hear changes pass (!). This is only a suggestion of course but I had coded it already and decided to submit a PR.
What about merging only changes to Control.py and Cluster.py?
That should mean that .cgat and .ini file defaults should run without the need to change anything.
More generally, what's are the thoughts/plans at the moment for refactoring? Would switching this PR to CGATCore be better for instance?
We could hold a second meeting if easier. I'd be keen on being able to continue working on CGAT code and external pipelines without too much worry. Happy to wait otherwise and run off a branch though.
Antonio
I think we could merge right now the changes for:
CGATPipelines/Pipeline/__init__.py
CGATPipelines/configuration/pipeline.ini
CGATPipelines/Pipeline/Cluster.py
and leave out the changes for Control.py
due to the incoming changes?
Are you happy with this @AntonioJBT ?
@AndreasHeger will announce it in due course -- pretty soon by the looks of it! ;)
@sebastian-luna-valero That's fine with me. Reverting for now then in a new commit and merging recent changes from master... Looking forward to @AndreasHeger announcement :+1:
Great, thanks @AntonioJBT
Before merging into the master branch, I just thought that adding to-set
to all parameters that need to be configured makes it easier to find where to change them with: grep to-set -r <clone-of-cgat>
Is that helpful?
Hi @sebastian-luna-valero Yes, that is probably an easy way to quickly check what needs setting. We'll probably need a more thorough check in the future to catch all of them though Thanks! Antonio
Hi @sebastian-luna-valero and @AntonioJBT , in the past we used !?
to mark parameters that need to be set manually.
Thanks @AndreasHeger
I have replaced to-set
with !?
in https://github.com/CGATOxford/CGATPipelines/commit/50430d271627ce702373db5fafd48dba27fdbcc0 to be consistent with the previous convention
Hi @sebastian-luna-valero, @Acribbs, @AndreasHeger, I've made a few changes to Control.py in order to:
Changes are potentially breaking. Could you please check carefully when possible?
Currently, we run pipelines with:
And possibly as:
This last one can be done easily if it is a python package with the correct entry point in its setup.py
I've tested changes with readqc and an external pipeline I'm working on:
pipeline_QTL
in a CentOS HPC and locally in a Mac.
The external pipeline can be installed with pip and ran as e.g.:
Most of its dependencies are already included in CGATPipelines. You might be missing r-docopt only. See:
https://raw.githubusercontent.com/EpiCompBio/pipeline_QTL/master/installation/Dockerfile
for detailed instructions. The Dockerfile itself is untested though.
Additionally, there are some very minor changes to make configuration files slightly more generic. These can be discarded. It may be a thought to have generic conf.py and pipeline.ini files if later transferring to CGAT core for instance.
Any thoughts, changes, errors, etc. please let me know.
Thanks! Antonio