Human-GEM
Human-GEM copied to clipboard
SysBioChalmers
feat: add gene essentiality check using DepMap
Main improvements in this PR:
Added evaluation of gene essentiality using DepMap data. Note that the current code only runs 2 chunks out of 40 (let's change this when producing figures for the paper). Also, it currently only runs it for the model in the repo - we will have to run it in the same way for an older model version and modify the code slightly to assemble the data from both runs. Note that this PR was based on the hart2015 PR since that code was needed before it was approved.
I hereby confirm that I have:
- [X] Tested my code on my own computer for running the model
- [X] Selected
developas a target branch - [X] Any removed reactions and metabolites have been moved to the corresponding deprecated identifier lists
looks good - except that it suppose to be started from develop rather than a PR in draft, better change this @johan-gson
looks good - except that it suppose to be started from
developrather than a PR in draft, better change this @johan-gson
I had to do that, I needed the code from that PR before it was approved. We did like this often in GECKO3. I'm thinking we can do like this?
Great job! I've left some aesthetic comments.
In terms of the bigger picture, what is the aim here? To be able to run this once, now and again? Or more often, say with every other release?
I'm thinking it is twofold - both at every release but also as a oneoff for generating a figure for the paper. In the second case, it takes some tweaking. This is why I left some commented out code, it will be useful for that purpose.
I had to do that, I needed the code from that PR before it was approved. We did like this often in GECKO3. I'm thinking we can do like this?
this is fine - To avoid confusion, maybe clarify in PR message that the code is based on that PR?
I had to do that, I needed the code from that PR before it was approved. We did like this often in GECKO3. I'm thinking we can do like this?
this is fine - To avoid confusion, maybe clarify in PR message that the code is based on that PR?
Done
Great job! I've left some aesthetic comments.
In terms of the bigger picture, what is the aim here? To be able to run this once, now and again? Or more often, say with every other release?
I would say with the larger essentiality check for DepMap data should be performed every major release in the main branch. But for each PR, then we can run a smaller scale test as Hao committed in https://github.com/SysBioChalmers/Human-GEM/pull/563
I would say with the larger essentiality check for DepMap data should be performed every major release in the main branch. But for each PR, then we can run a smaller scale test as Hao committed in https://github.com/SysBioChalmers/Human-GEM/pull/563
sounds a reasonable plan given the heavy computation load of DeepMap data
sounds a reasonable plan
Great! Perhaps this should be added to https://github.com/SysBioChalmers/Human-GEM/wiki/How-to-make-a-new-release so as not to forget about this step.
Very nice, @mihai-sysbio, I had not seen this before. It solves the problem!
Through the previous commit a07b7b6, the file Instructions.txt was removed. Instead, a reformatted set of instructions has been drafted for addition in the Human-GEM-guide PR https://github.com/SysBioChalmers/Human-GEM-guide/pull/10.
Great! Perhaps this should be added to https://github.com/SysBioChalmers/Human-GEM/wiki/How-to-make-a-new-release so as not to forget about this step.
✅