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

Addition of massbank records in folder AU #33

Closed
wants to merge 88 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@nalygizakis

nalygizakis commented Dec 3, 2018

Hi Tobias,

I forked the repository, added the new MassBank records and I created a pull request to merge the directories.

Kind Regards,
Nikiforos

nalygizakis added some commits Dec 2, 2018

nalygizakis added some commits Dec 3, 2018

@meier-rene

This comment has been minimized.

Collaborator

meier-rene commented Dec 3, 2018

Hi Nikiforos,
thank you for the pull request. I would like to help you getting these new records into MassBank. I will check the validation results and come back with some suggestions.

@nalygizakis

This comment has been minimized.

nalygizakis commented Dec 3, 2018

Thanks! Please, go ahead and let me know if there is any validation problem.

@meier-rene

This comment has been minimized.

Collaborator

meier-rene commented Dec 5, 2018

I had a look into your submission and I found a number of issues which I can not solve without your help. I could probably solve a lot of this on my own, but I would feel more comfortable if you at least know it and approve these things. Even better would be if you try to reprocess them all with a recent RMassBank e.g. from Bioconductor 3.8, because this would solve a number of issues automatically.

Issue 1:
As far as I understand a lot of the new records were created with a normal phase chromatography and you labeled them LC-HILIC but in MassBank we only have LC, GC and and CE-MS. These records would need a relabeling to LC-ESI-QTOF.
Issue 2:
The ACCESSION in the files do not match the filenames. I could set the ACCESSION to the filename. Setting the filename to the ACCESSION is not possible, because the filenames are already occupied .

There are more minor issues, but before we discuss this I would like to know if you prefer to solve Issue 1 & 2 on your own with a new RMassBank. If you think you can not manage this I need advice for Issue 2.

@nalygizakis

This comment has been minimized.

nalygizakis commented Dec 5, 2018

Hi! Good news is that issues are clear. I will correct both issues myself.

I have a question for you; I understood that AC$INSTRUMENT_TYPE should be LC-ESI-QTOF. However, is it possible that RECORD_TITLE is "Compound name; LC-HILIC-ESI-QTOF; ..."?

I renamed the records, so that they do not conflict with RP records. This is why you have ACCESSION conflict.

@schymane

This comment has been minimized.

Member

schymane commented Dec 5, 2018

@nalygizakis

This comment has been minimized.

nalygizakis commented Dec 5, 2018

Hi Emma. Let's take an example so that it is clear to me.
RECORD_TITLE: 1H-Benzotriazole; LC-HILIC-ESI-QTOF; MS2; CE: 20 eV; R=35000; [M+H]+
is suggested to be changed to
RECORD_TITLE: 1H-Benzotriazole; LC-ESI-QTOF; HILIC; MS2; CE: 20 eV; R=35000; [M+H]+

Do you agree with the example above?

@schymane

This comment has been minimized.

Member

schymane commented Dec 5, 2018

@meier-rene

This comment has been minimized.

Collaborator

meier-rene commented Dec 5, 2018

Any chance to put this a little bit more to the end of the line? Would make things more easy in MassBank code. The first three fields are validated atm. Could you agree to
RECORD_TITLE: 1H-Benzotriazole; LC-ESI-QTOF; MS2; HILIC; CE: 20 eV; R=35000; [M+H]+
?

@nalygizakis

This comment has been minimized.

nalygizakis commented Dec 5, 2018

It is feasible to follow this set up. Therefore, we cancel the pull request and I will fork again, upload the new records and try to pull again. Do you agree @meier-rene?

@meier-rene

This comment has been minimized.

Collaborator

meier-rene commented Dec 5, 2018

Would be great.

@meier-rene

This comment has been minimized.

Collaborator

meier-rene commented Dec 5, 2018

Waiting for new try...

@meier-rene meier-rene closed this Dec 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment