-
Notifications
You must be signed in to change notification settings - Fork 42
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/duplicated components #213
Conversation
data/annotation/metabolites.tsv
Outdated
@@ -451,7 +451,7 @@ mets metsNoComp metBiGGID metKEGGID metHMDBID metChEBIID metPubChemID metLipidMa | |||
"m00242s" "m00242" "" "C11088" "" "" "" "" "" "" "M00242" "MNXM6026" "m00242s" | |||
"m00243c" "m00243" "" "C06205" "" "CHEBI:28516" "" "" "" "" "M00243" "MNXM114239" "m00243c" | |||
"m00244c" "m00244" "" "C14784" "" "CHEBI:34048" "" "" "" "" "M00244" "MNXM9529" "m00244c" | |||
"m00245c" "m00245" "" "C15606" "" "CHEBI:49252" "" "" "" "" "M00245" "MNXM494" "m00245c" | |||
"m00245c" "m00245" "dhmtp" "C15606" "" "CHEBI:49252" "" "" "" "" "M00245;dhmtp_c" "MNXM494" "m00245c" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find metabolite dhmtp_c
in Recon3D.
data/annotation/metabolites.tsv
Outdated
@@ -999,7 +999,7 @@ mets metsNoComp metBiGGID metKEGGID metHMDBID metChEBIID metPubChemID metLipidMa | |||
"m00602r" "m00602" "" "" "" "" "53481513" "" "CE3554" "" "CE3554" "MNXM35505" "m00602r" | |||
"m00603c" "m00603" "" "C05497" "" "" "" "" "" "" "M00603" "MNXM3472" "m00603c" | |||
"m00604c" "m00604" "" "C13713" "HMDB00879" "CHEBI:805752" "101771" "" "CE5072" "" "CE5072" "MNXM8277" "m00604c" | |||
"m00605c" "m00605" "" "C05485" "" "CHEBI:28043" "" "LMST02030167" "" "" "M00605" "MNXM4305" "m00605c" | |||
"m00605c" "m00605" "21hprgnlone" "C05485" "" "CHEBI:28043" "" "LMST02030167" "" "" "M00605;21hprgnlone_c" "MNXM735991" "m00605c" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here with this other Recon3D metabolite id. And I've seen the same on most of the other commits, but I haven't checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mihai-sysbio This and the other ID are present in the Recon3D model (and "reconstruction"). The problem is that on VMH, they strip off the compartment abbreviation (_c
). The original aim was to use these Recon3D
fields to help maintain compatibility with the Recon3D GEM. However, since these Recon3D IDs are supposed to link to VMH, maybe we should consider stripping off the compartment abbreviation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. How probable is the scenario of one needing to map to Recon3D specifically, and not VMH? If the Recon3D ids are needed, I would suggest renaming the current column to VMH and adding a new one, that is not supposed to have a valid link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's probably better to remove the compartment abbreviations and have them properly link to VMH. If someone is interested in re-mapping to the Recon3D model, they could simply append the compartment abbreviation to all the "Recon3D" (VMH) IDs.
Based on @mihai-sysbio's good suggestion, I have now removed the compartment abbreviations from all |
- Provide high confidence scores for reactions HMR_5387, HMR_5389, HMR_8798 that had been curated with new literature
@JonathanRob very nice work! The fixing of duplicated metabolites also involves merging of duplicated reactions. In the following 4 cases: maltohexaose: To be consistent with above replacement, how about also renaming the corresponding exchange reactions as such: from |
Thank you, @Hao-Chalmers.
Since we are anyway planning to do a complete re-naming of the reaction IDs (#174), I don't think it is worth changing the names of these reactions at this point. Re-naming them now will just disrupt backwards compatibility. |
ah yes, can't believe I forgot this. |
Main improvements in this PR:
This PR primarily addresses Issue #184, which identified several metabolites with identical names except for a difference in capitalization.
Each set of duplicated metabolites was merged, and the duplicate entry (or entries) removed. Since the merger of each metabolite often resulted in one or more reactions becoming identical, all reactions in which the affected metabolites were involved were manually inspected to ensure proper metabolite replacement and removal of new duplicate reactions. Each duplicate metabolite was addressed in a separate commit, to clarify the changes to the GEM associated with merging the metabolite set. In addition, the several reaction and metabolite annotations (i.e., external identifiers) were updated during the process.
Additional changes:
The
others
reaction and metabolite was removed from Human-GEM (addresses Issue #197), as well as any metabolites that occurred solely in that reaction. This was a pool reaction originating from HMR2 that no longer served any foreseeable purpose, and was anyway unable to carry flux (dead-end).The GEM annotation files (
reactions.tsv
,metabolites.tsv
, andgenes.tsv
) were moved from thedata/annotation/
subdirectory to themodel/
directory for convenience (suggested in comments of PR #212). All functions/documentation relying on the location of these files was also updated.I hereby confirm that I have:
devel
as a target branch