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

(re) example of correction to apply in order to avoid conflict during MNXref conversion #358

Merged
merged 12 commits into from
Mar 1, 2022

Conversation

ANiknejad
Copy link
Contributor

@ANiknejad ANiknejad commented Feb 3, 2022

(sorry, I closed the previous pull request, because I thought I can choose the commits I want to show you, but now I have understand how it works...(I hope so ;-) ; anyway sorry for lot of pointless commits)

Hello,

I show you here an example of the correction we can do on the 'metabolites.tsv' file,
thanks to the information retrieved using the status codes generated during the conversion of the 'metatlas_HumanGEM' model to MNXref.

See the public documentation here:
https://github.com/MetaNetX/MNXtools/blob/main/doc/convert_manual.md

Here the case where the source id has conflicting xrefs, one xref is for a precise structure, the other is for a generic structure: depending on the source id, one should be removed

MetaNetX report file, we got

CHEM_XREF_CONFLICT MAM00097 (5-L-glutamyl)-L-amino acid
keggC:C03740 MNXM730664 (generic structure)
biggM:gluala MNXM1105815 (precise structure)
UNK:MAM00097

MNXM1105815 = chebi:50619 L-gamma-glutamyl-L-alanine
MNXM730664 = chebi:71304 an alpha-(gamma-L-glutamyl)-L-amino acid

First to do: check the name and structure and associated reactions for the source compound, here MAM00097 (5-L-glutamyl)-L-amino acid
https://metabolicatlas.org/explore/Human-GEM/gem-browser/metabolite/MAM00097s

Here the name is for a generic structure (-L-amino acid) but the associated chebi and the associated reactions are those for the defined
L-gamma-glutamyl-L-alanine

in the HUMAN_GEM 'metabolites.tsv' file, MAM00097s [(5-L-glutamyl)-L-amino acid] has xref: "gluala" "C03740" "HMDB06248" "CHEBI:50619'

gluala (bigg) is patched to chebi:50619 (L-gamma-glutamyl-L-alanine) and the xref here is correct, to report the compound that does the described reactions
C03740 (kegg) is patched to chebi:71304 (an alpha-(gamma-L-glutamyl)-L-amino acid)) and this structure is the generic form: we have to remove the xref
hmdb:HMDB0006248 is patched to chebi:50619 (L-gamma-glutamyl-L-alanine) and the xref here is correct, ok
CHEBI:50619: the precise structure, ok

to resolve this conflict:

by removing xref: C03740 (done in this commit)

I hereby confirm that I have:

  • Selected develop as a target branch

@ANiknejad
Copy link
Contributor Author

hello Team,
@haowang-bioinfo

May I ask you to review this PR? I am sorry in advance, many commits for at the end few modifications into the metabolites.tsv file, but anyway these changes should improve the integration of HumanGEM into MNXref common namespace.

Best regards,

Anne

Copy link
Collaborator

@JonathanRob JonathanRob left a comment

Choose a reason for hiding this comment

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

Looks good - thanks again for your help! I suggested a few changes you could make on your end to update the metabolite identifiers (which have changed on our version of develop during your work on your version). Once these changes are made, then it should help to solve the conflicts that currently exist between these branches.

model/metabolites.tsv Outdated Show resolved Hide resolved
model/metabolites.tsv Outdated Show resolved Hide resolved
model/metabolites.tsv Outdated Show resolved Hide resolved
Copy link
Collaborator

@JonathanRob JonathanRob left a comment

Choose a reason for hiding this comment

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

Thanks, @ANiknejad. I think it should now be OK to merge these into develop (using squash & merge, and --ours?) but I will ask for @mihai-sysbio's help here, since he is more experienced with git.

@mihai-sysbio
Copy link
Member

Squash and merge should do the trick indeed @JonathanRob. That will combine all the commits into a single one, making the changes easier to understand. Sometimes this is very useful, for example in this PR, where changes were split in different commits.
There seem to be 4 conflicts left, and they are small (just a few lines), but without knowing the details it's hard for me to know which are the correct lines. @JonathanRob are you suggesting to favor all the changes in this branch over the ones in develop?

@JonathanRob
Copy link
Collaborator

@mihai-sysbio Ok, thanks. Yes, the squash portion makes sense, it was just the "favoring changes in this branch over those in develop" that I am hesitant about.

The lines/rows in metabolites.tsv in this branch that contain changes are correct, and should be favored. However, when I view the conflicts here in GitHub (using e.g. the Resolve conflicts button), it seems that in some cases it is including lines above/below those with changes (some of which are outdated and should not be favored). So I am uncertain if merging will also bring these "surrounding" lines along.

@mihai-sysbio
Copy link
Member

mihai-sysbio commented Feb 28, 2022

It was unexpectedly tricky to resolve this - thanks for the heads up @JonathanRob. I ended up using Resolve conflicts and checking manually line-by-line which change should be taken in. The new changes in this PR were good to go. Some of the surrounding lines were caused by old compartment identifiers, which I cross-checked against the PR where these letters were last updated #341 (comment), thus resulting in the above commit 14d2bb0 into ANiknejad:develop.
The merge appears ready now, and as far as I can see all @ANiknejad's changes are still there. I'm leaving this to @JonathanRob, in case you want to have one last look.

@JonathanRob
Copy link
Collaborator

Great, thanks @mihai-sysbio for your help on this.

@JonathanRob JonathanRob merged commit 11186ac into SysBioChalmers:develop Mar 1, 2022
@haowang-bioinfo
Copy link
Member

@ANiknejad sorry for the late response. Everything looks great with the excellent work of @mihai-sysbio and @JonathanRob

@mihai-sysbio mihai-sysbio mentioned this pull request Jun 17, 2022
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