Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upAddition of massbank records in folder AU #33
Conversation
nalygizakis
added some commits
Dec 2, 2018
nalygizakis
added some commits
Dec 3, 2018
This comment has been minimized.
This comment has been minimized.
Hi Nikiforos, |
This comment has been minimized.
This comment has been minimized.
nalygizakis
commented
Dec 3, 2018
Thanks! Please, go ahead and let me know if there is any validation problem. |
This comment has been minimized.
This comment has been minimized.
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: 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. |
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
You can modify the Record Title pattern in the settings file … I would suggest you add HILIC after the actual instrument code tho … e.g. … LC -ESI-QTOF; HILIC; ….
|
This comment has been minimized.
This comment has been minimized.
nalygizakis
commented
Dec 5, 2018
•
Hi Emma. Let's take an example so that it is clear to me. Do you agree with the example above? |
This comment has been minimized.
This comment has been minimized.
Looks fine to me – it will get you HILIC in the title but won’t interfere with the automatic fields. You may need to check with the Record Specification but I do not see anything to prevent this (we e.g. added Resolution into the title), as far as I recall they have a minimum information they want in the title but do not prevent you adding more. Pls correct me if I am wrong @meier-rene as I’ve not had a chance to check for sure.
|
This comment has been minimized.
This comment has been minimized.
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 |
This comment has been minimized.
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? |
This comment has been minimized.
This comment has been minimized.
Would be great. |
This comment has been minimized.
This comment has been minimized.
Waiting for new try... |
nalygizakis commentedDec 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