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.

Sign up for GitHub

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

Jump to bottom

fix: remove non-transporter genes #448

Closed
wants to merge 1 commit into from
Closed

fix: remove non-transporter genes #448

wants to merge 1 commit into from

Conversation

haowang-bioinfo
Copy link
Member

@haowang-bioinfo haowang-bioinfo commented Jan 9, 2023 •
edited
Loading

Main improvements in this PR:

I hereby confirm that I have:

  • Tested my code on my own computer for running the model
  • Selected develop as a target branch

@haowang-bioinfo haowang-bioinfo marked this pull request as draft January 9, 2023 18:47
@haowang-bioinfo
Copy link
Member Author

Note that removal of CLTRN (ENSG00000147003) and SLC6A18 (ENSG00000164363) in this PR led to 14 amino acid transport reactions with empty GPR rules.

These 14 reactions are retained as such. Further changes will be conducted when actual transporters are identified.

@haowang-bioinfo haowang-bioinfo marked this pull request as ready for review January 10, 2023 09:26
johan-gson
johan-gson reviewed Jan 10, 2023
View reviewed changes
model/Human-GEM.yml
@@ -172044,7 +172044,7 @@
- MAM02751c: 1
- lower_bound: 0
- upper_bound: 1000
- gene_reaction_rule: "(ENSG00000101892 and ENSG00000137731 and ENSG00000163399) or (ENSG00000018625 and ENSG00000129244 and ENSG00000137731) or (ENSG00000018625 and ENSG00000069849 and ENSG00000137731) or (ENSG00000101892 and ENSG00000105409 and ENSG00000137731) or (ENSG00000105409 and ENSG00000137731 and ENSG00000143153) or (ENSG00000105409 and ENSG00000129244 and ENSG00000137731) or (ENSG00000069849 and ENSG00000105409 and ENSG00000137731) or (ENSG00000101892 and ENSG00000132681 and ENSG00000137731) or (ENSG00000132681 and ENSG00000137731 and ENSG00000143153) or (ENSG00000129244 and ENSG00000132681 and ENSG00000137731) or (ENSG00000069849 and ENSG00000132681 and ENSG00000137731) or (ENSG00000137731 and ENSG00000143153 and ENSG00000163399) or (ENSG00000129244 and ENSG00000137731 and ENSG00000163399) or (ENSG00000069849 and ENSG00000137731 and ENSG00000163399) or (ENSG00000018625 and ENSG00000101892 and ENSG00000137731) or (ENSG00000018625 and ENSG00000137731 and ENSG00000143153)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The first and you have removed here contains ENSG00000137731, which is not in the remove list in issue 444. I'm not sure if the other two are signaling enzymes that activates this enzyme, or how this works, but please check that this is correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The same goes for ENSG00000163399. Looking this over is probably good.

johan-gson
johan-gson reviewed Jan 10, 2023
View reviewed changes
model/Human-GEM.yml
@@ -172506,7 +172506,7 @@
- MAM02751c: 1
- lower_bound: 0
- upper_bound: 1000
- gene_reaction_rule: "(ENSG00000101892 and ENSG00000137731 and ENSG00000163399) or (ENSG00000018625 and ENSG00000129244 and ENSG00000137731) or (ENSG00000018625 and ENSG00000069849 and ENSG00000137731) or (ENSG00000101892 and ENSG00000105409 and ENSG00000137731) or (ENSG00000105409 and ENSG00000137731 and ENSG00000143153) or (ENSG00000105409 and ENSG00000129244 and ENSG00000137731) or (ENSG00000069849 and ENSG00000105409 and ENSG00000137731) or (ENSG00000101892 and ENSG00000132681 and ENSG00000137731) or (ENSG00000132681 and ENSG00000137731 and ENSG00000143153) or (ENSG00000129244 and ENSG00000132681 and ENSG00000137731) or (ENSG00000069849 and ENSG00000132681 and ENSG00000137731) or (ENSG00000137731 and ENSG00000143153 and ENSG00000163399) or (ENSG00000129244 and ENSG00000137731 and ENSG00000163399) or (ENSG00000069849 and ENSG00000137731 and ENSG00000163399) or (ENSG00000018625 and ENSG00000101892 and ENSG00000137731) or (ENSG00000018625 and ENSG00000137731 and ENSG00000143153)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing here

johan-gson
johan-gson reviewed Jan 10, 2023
View reviewed changes
model/Human-GEM.yml
@@ -249224,8 +249224,8 @@
- MAM02519c: 1
- MAM02519e: -1
- lower_bound: 0
- upper_bound: 1000
- gene_reaction_rule: "ENSG00000147003 and ENSG00000164363"
Copy link
Collaborator

Choose a reason for hiding this comment

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

ENSG00000147003 is not listed in issue 444.

johan-gson
johan-gson reviewed Jan 10, 2023
View reviewed changes
model/Human-GEM.yml
@@ -249264,8 +249264,8 @@
- MAM02519c: 1
- MAM02519e: -1
- lower_bound: 0
- upper_bound: 1000
- gene_reaction_rule: "ENSG00000147003 and ENSG00000164363"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

johan-gson
johan-gson reviewed Jan 10, 2023
View reviewed changes
model/Human-GEM.yml
@@ -249284,8 +249284,8 @@
- MAM02519c: 1
- MAM02519e: -1
- lower_bound: 0
- upper_bound: 1000
- gene_reaction_rule: "ENSG00000147003 and ENSG00000164363"
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here

johan-gson
johan-gson reviewed Jan 10, 2023
View reviewed changes
model/Human-GEM.yml
@@ -249362,8 +249362,8 @@
- MAM02519c: 1
- MAM02519e: -1
- lower_bound: 0
- upper_bound: 1000
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here

Copy link
Collaborator

Choose a reason for hiding this comment

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

And a whole bunch of places further down

johan-gson
johan-gson reviewed Jan 10, 2023
View reviewed changes
Copy link
Collaborator

@johan-gson johan-gson left a comment

Choose a reason for hiding this comment

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

Can you take a look at the comments I added - I some of the removed genes are not in the removal list in issue 444.

@haowang-bioinfo
Copy link
Member Author

good job @johan-gson, you correctly pinpointed the mistakes I made, now close this to fix the problme.

@haowang-bioinfo
Copy link
Member Author

The removal of ENSG00000137731 and ENSG00000163399 is due to ENSG00000101892, while ENSG00000147003 was removed because of ENSG00000164363. They are subunits of two complexes that are striped away as a whole when any subunit is deleted.

To avoid this unexpected outcome, ENSG00000101892 and ENSG00000164363 are taken away from the list in #444, while the updated change is implemented afresh in #450.

@haowang-bioinfo haowang-bioinfo mentioned this pull request Jan 10, 2023
Closed
2 tasks
@haowang-bioinfo haowang-bioinfo deleted the fix/delNonTransporterGenes branch January 13, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johan-gson johan-gson johan-gson left review comments

@feiranl feiranl Awaiting requested review from feiranl

@JonathanRob JonathanRob Awaiting requested review from JonathanRob

@mihai-sysbio mihai-sysbio Awaiting requested review from mihai-sysbio

Assignees
No one assigned
Labels
AI-assisted curation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@haowang-bioinfo @johan-gson