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

Add transport rxn GPR #178

Merged
merged 8 commits into from
Dec 6, 2018
Merged

Add transport rxn GPR #178

merged 8 commits into from
Dec 6, 2018

Conversation

feiranl
Copy link
Collaborator

@feiranl feiranl commented Nov 17, 2018

Main improvements in this PR:

  • Addressing issue feat: add gene annotation for transport reactions #171:
  • This PR is mainly for adding GPR rules to transport rxns according to different databases, mainly TCDB annotation. All information are stored in the file of TransRxnGeneAnnotation.tsv
  • addTransNewGPR.m is the function that we used to adding new GPRs to rxns.
  • Reaction confidence scores were also updated with addConfidenceScores.m (leading to changes in some unexpected rxns due to old PRs not updating them).

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

* Add new GPRs for transport rxns
* Add a function for updating those GPRs to model
* Update new GPRs for those transport rxns
@BenjaSanchez BenjaSanchez self-assigned this Nov 26, 2018
@BenjaSanchez BenjaSanchez added enhancement new field/feature curation labels Nov 26, 2018
Copy link
Contributor

@BenjaSanchez BenjaSanchez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feiranl the changes look good. My only comment is that if you are adding gene rules, then the corresponding reactions should now have a reaction confidence score = 2 (if they don't have pmid) or = 3 (if they have pmid). For updating the model with this you could try running addConfidenceScores.m, but make sure that no undesired changes occur (i.e. only these rxns change).

  • Compatibility: no unexpected errors when the model goes through an I/O cycle.
  • Documentation: The corresponding scripts are well documented.
  • Reproducibility: All changes introduced to the model can be reproduced with the corresponding scripts.
  • Style: Compliant style in the model/functions/data.

@BenjaSanchez BenjaSanchez added the wip work in progress label Nov 26, 2018
@feiranl
Copy link
Collaborator Author

feiranl commented Nov 28, 2018

Okay, will change the confidence score today.

@feiranl
Copy link
Collaborator Author

feiranl commented Nov 30, 2018

I used this function, and found also confidence score of other rxns will also change.

Update the confidence scores for those reactions of transport rxns. Note: r_0475, r_1667, r_1729, r_1805, r_2025, r_2064, r_4403 ,r_4435, r_4484, r_4598, r_4599 rxn confidence scores were also changed due to they have gRrules
Copy link
Contributor

@BenjaSanchez BenjaSanchez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feiranl everything looks good now! after your latest commit using addConfidenceScores.m I see that some rxn confidence scores not from this PR are also getting modified, such as r_0475 or r_1667. However, those changes are correct and likely to have been forgotten in previous PRs (e.g. #124), so for simplicity we can integrate them here as well (I included this observation in the PR description above).

@feiranl @hongzhonglu note that this PR will be merged after the next release, to separate it from #174

@BenjaSanchez BenjaSanchez added standby work somewhere else is needed before advancing and removed wip work in progress labels Dec 3, 2018
@feiranl
Copy link
Collaborator Author

feiranl commented Dec 3, 2018

I include this note in last commit, confidence scores for those rxns which don't belong to this PR were also changed :
Note: r_0475, r_1667, r_1729, r_1805, r_2025, r_2064, r_4403 ,r_4435, r_4484, r_4598, r_4599 rxn confidence scores were also changed since they have gRrules.

@BenjaSanchez BenjaSanchez merged commit da7dcec into devel Dec 6, 2018
@BenjaSanchez BenjaSanchez deleted the AddTransGPR branch December 17, 2018 15:04
@BenjaSanchez BenjaSanchez mentioned this pull request May 22, 2019
@BenjaSanchez BenjaSanchez mentioned this pull request May 25, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new field/feature standby work somewhere else is needed before advancing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants