LC-QTOF-MS - Githubissues

MassBank / MassBank-data

Official repository of open data MassBank records

74 stars 59 forks source link

LC-QTOF-MS #103

Closed raalizadeh closed 4 years ago

raalizadeh commented 4 years ago

Bisphenol derivatives

meier-rene commented 4 years ago

@raalizadeh

https://github.com/tsufz/MassBank-web/blob/update_record_format/Documentation/MassBankRecordFormat.md

more easy to read version of the document can be found here.

raalizadeh commented 4 years ago

I will close this commit and add new files. I wrote some functions to modify the text files and adding some of these tags... Sorry for the inconvenience, I did not notice that we have got new template for the MassBank record.

raalizadeh commented 4 years ago

Hey guys, I fixed the LC-ESI-QTOF and GC-APCI-QTOF data, but whenever I create pull request, as Tobias said above, it keeps saying that a file is missing. Any idea what causes it?

schymane commented 4 years ago

Hi @raalizadeh thanks for your contributions! I've just had a look at the travis report, I can't see the missing file error but it looks like ION_MODE is missing, can you check how this should be written and correct? e.g. https://travis-ci.org/MassBank/MassBank-data/jobs/616223930#L2663

raalizadeh commented 4 years ago

Thanks Reza! Let's wait and see what Travis says ..

It seems like it has strict checkpoint and the "order of the tags" should be preserved. It now asks for SPLASH which is already there... but I think that SPLASH should come after chromatographic info... Any idea?

schymane commented 4 years ago

Indeed the build fails again with errors and warnings but PK$SPLASH appears to be in the right place to me, also consistent with other records (top of PK$ section). @meier-rene any ideas what is going wrong here? e.g. https://travis-ci.org/MassBank/MassBank-data/jobs/616239382#L2418 vs https://github.com/MassBank/MassBank-data/commit/7a1e1f0b331f630b0f874ebc6d4690fb210d2cd4#diff-1f3a027e953f969d159efd4b4dce961aL45

Also @meier-rene @tsufz there are warnings re the use of the new fields .. can you confirm Reza is using these correctly and update the validator? The warnings and errors start here: https://travis-ci.org/MassBank/MassBank-data/jobs/616239382#L2415

schymane commented 4 years ago

Yes, now we have an interesting SPLASH bug - we are discussing by email. https://travis-ci.org/MassBank/MassBank-data/jobs/616251567#L2418 For some reason one block is missing, which does not make sense according to latest RMB: https://github.com/MassBank/RMassBank/blob/master/R/getSplash.R

schymane commented 4 years ago

Thanks for the contribution @raalizadeh - glad to see you could fix the SPLASH issue!

raalizadeh commented 4 years ago

Thanks for the contribution @raalizadeh - glad to see you could fix the SPLASH issue!

Thank you @schymane for the quick feedback... Yes, it is fixed (updated all the r packages...hehe)

tsufz commented 4 years ago

@meier-rene, hence, will there be a release now? If not, I will update from dev.

schymane commented 4 years ago

There is still one more outstanding pull request that someone (@tsufz ?) should look at before a release: https://github.com/MassBank/MassBank-data/pull/104