Skip to content
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

fix-rxn-prop:addNewGPRrelations #124

Merged
merged 11 commits into from
Jun 26, 2018
Merged

fix-rxn-prop:addNewGPRrelations #124

merged 11 commits into from
Jun 26, 2018

Conversation

feiranl
Copy link
Collaborator

@feiranl feiranl commented Jun 20, 2018

Main improvements in this PR:

  • This improvement is based on genome annotation for Saccharomyces cerevisiae S288c from five databases (SGD, Uniprot, KEGG, Biocyc and Reactome). Reactions linked to these new annotated genes were extracted from those databases (Uniprot, KEGG, Biocyc and Reactome) and by searching annotated EC numbers in different databases (Rhea and Brenda).
  • Add database new annotation detailed information in ComplementaryData/databases/DBnewGeneAnnotation.tsv.
  • Add new GPR relations for existing reactions according to database new annotation.

Addresses issue #60

I hereby confirm that I have:

  • Tested my code with all requirements for running the model
  • Selected devel as a target branch (top left drop-down menu)
  • If needed, asked first in the Gitter chat room about this PR

feiranl and others added 5 commits June 20, 2018 11:18
add database new gene annotation files
Add new GPR relations for existing reactions
no content changes
* geneName = geneID when geneName in SGD DB has no match or is empty
* should be "COBRAProtein"
@edkerk
Copy link
Member

edkerk commented Jun 21, 2018

Structurally it looks fine, but it would good to see a brief description on how these GPRs were decided. Doesn't have to be very detailed, but now this is entirely unclear. Can be mentioned in this PR, or could have been in commit messages, but now it is not mentioned anywhere.

@BenjaSanchez
Copy link
Contributor

BenjaSanchez commented Jun 21, 2018

@edkerk the reasoning behind each change can be found in DBnewGeneAnnotation.tsv (sadly too big to be properly seen on Github, but you can open it locally with Excel), which contains all new genes added. For instance, rxn r_0005 has an additional gene YMR306W in the gene rule. If you go to DBnewGeneAnnotation.tsv and search for YMR306W, you'll find that it catalyzes indeed that reaction according to all databases used. The same can be done for all other changes.

@edkerk
Copy link
Member

edkerk commented Jun 21, 2018

As discussed, please provide a short description of where the information regarding gene annotations is coming from, by editing the original pull request message.

feiranl and others added 2 commits June 21, 2018 17:20
change column title for the DBNewGeneAnnotation
@hongzhonglu
Copy link
Collaborator

@feiranl could you check 'YAR073W', which may be pseudogene? We should remove gene like this to make sure the quality.

feiranl and others added 4 commits June 25, 2018 16:38
YAR0723W ,  YIL168W and YIL167W are not functional
delete three non functional genes and changed the related GPR rules
Delete unsed 3 genes using cobra function:model = removeUnusedGenes(model)
@BenjaSanchez
Copy link
Contributor

@feiranl @hongzhonglu any remaining changes? if none I will merge in the afternoon.

@hongzhonglu
Copy link
Collaborator

@BenjaSanchez No changes from my side.

@feiranl
Copy link
Collaborator Author

feiranl commented Jun 26, 2018

@BenjaSanchez No changes from my side.

@BenjaSanchez BenjaSanchez merged commit cf2da24 into devel Jun 26, 2018
@BenjaSanchez BenjaSanchez mentioned this pull request Jun 26, 2018
@BenjaSanchez BenjaSanchez deleted the fix/DBnewAnnotation branch July 7, 2018 10:48
@feiranl feiranl mentioned this pull request Jul 20, 2018
3 tasks
@BenjaSanchez BenjaSanchez mentioned this pull request Aug 30, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants