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

Proposed changes to Sodium Transport GPRs #569

Closed
7 tasks done
jreimertz opened this issue May 17, 2023 · 7 comments
Closed
7 tasks done

Proposed changes to Sodium Transport GPRs #569

jreimertz opened this issue May 17, 2023 · 7 comments
Labels

Comments

@jreimertz
Copy link
Collaborator

jreimertz commented May 17, 2023

I came across another collection of transport reactions which likely should should have their GPRs updated.

  • 1. Remove ENSG00000111371 (SLC38A1), ENSG00000134294 (SLC38A2), and ENSG00000139209 (SLC38A4) from reaction MAR06380

    • These all encode sodium-coupled amino acid transporters and do not include the H+ that is included in MAR06380
  • 2. Add ENSG00000188338 (SLC38A3) to MAR06380

    • SLC38A3 does encode the correct reaction and stoichiometry and should be added
  • 3. Remove reaction MAR05310

    • MAR05310 currently includes SLC38A3 and SLC38A5 in its GPR but this isn't the correct stoichiometry according to both Uniprot pages. Adding SLC38A3 to MAR06380 will fix this and remove the need for MAR05310.
  • 4. Add ENSG00000268104 (SLC6A14) to reaction MAR09887

    • The stoichiometry matches for SLC6A14 but it is currently not included in the GPR for MAR09887
  • 5. Remove ENSG00000164363 (SLC6A18) and ENSG00000147003 (CLTRN) from reaction MAR09887

    • SLC6A18 and CLTRN are the sodium dependent transporter components and do not involve Cl- which is included in MAR09887
  • 4. Remove MAR09887 because it does not match the reactions catalyzed by either gene it's currently associated with, and almost represents the reaction catalyzed by SLC6A14 (ENSG00000268104), but MAR06896 already represents that reaction with the correct stoichiometry and is already associated with SLC6A14

  • 5. Make MAR06380 reversible to match Uniprot annotation for SLC38A3

@jreimertz jreimertz added the bug label May 17, 2023
@jreimertz
Copy link
Collaborator Author

The proposed changes to MAR06380 and MAR05310 can also be applied to MAR06382 and MAR05312 respectively. I'll continue to add similar reactions as I encounter them

@haowang-bioinfo
Copy link
Member

haowang-bioinfo commented May 19, 2023

I'll continue to add similar reactions as I encounter them

good job - please continue!

As a side note, it's generally good to have issues and PRs with specific requests and changes. So instead of expanding existing issues/PRs, one can simply start new ones. This distinct separation helps in discussion, drawing a conclusion, figuring out solution, and making implementation. On the contrary, accumulating a lot of information in a single issue often makes things complex and difficult to resolve, as a result delay things from getting better.

A question naturally arises here is how big should an issue be? It's very hard to have a quantitive answer. From a reviewer's perspective, the amount of attention required to understand, evaluate the issue should definitely be less than 2 hours, the less the better. Small issues would lead to faster response, easier implementation, more importantly less errors.

This is not to exclude general discussions which are always welcome. But it's highly recommended to be as specific as possible when addressing an issue.

@haowang-bioinfo
Copy link
Member

@all-contributors please add @jreimertz for bug report.

@allcontributors
Copy link
Contributor

@haowang-bioinfo

I've put up a pull request to add @jreimertz! 🎉

@haowang-bioinfo
Copy link
Member

haowang-bioinfo commented Jul 4, 2023

@jreimertz sorry for the late response
@Devlin-Moyer here are a few comments to the changes in #655:

  • agree to remove MAR05310 and modify GPRs of MAR06380 as proposed. According to the Uniprot annotation, should MAR06380 be converted to reversible?
  • likewise, how about remove MAR09887 and keep MAR06896, which has matched stoichiometry and was assigned with SLC6A14?

@Devlin-Moyer
Copy link
Collaborator

yep it does look like MAR06380 should be reversible and that the proposed changes to MAR09887 would make it identical to MAR06896, so MAR09887 should just be removed altogether

@Devlin-Moyer
Copy link
Collaborator

Fixed in #655

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants