search

rformassspectrometry / Spectra

Low level infrastructure to handle MS spectra
https://rformassspectrometry.github.io/Spectra/
34 stars 24 forks source link

Master upstream #148

Closed jorainer closed 3 years ago

jorainer commented 3 years ago

I had a conflict between my local master and the Bioconductor master and was wondering why - seems that these changes here are in Bioconductor master, but not in our github master. I think we should revise this and check if these changes are really necessary - and in addition we need to sync the github and Bioconductor master

lgatto commented 3 years ago

I don't remember of these lines were to be removed, but this was part of the discussion whether mz and intensity where part of the spectra variables and whether they should be returned as part of spectraData().

Weird re out of sync of Github and Bioc - this should indeed never happen :-/

jorainer commented 3 years ago

In fact mz and intensity should not be removed from the .SPECTRA_DATA_COLUMNS, these are used in the validation functions of the MsBackendDataFrame object. Also, mz and intensity have to be part of the spectraData DataFrame returned from a MsBackend - always, because that is used to transfer the data if the backend is changed.

The Spectra object should by default not report these columns. But that's in the Spectra object, not in the MsBackend.

What I will do is a) re-add mz and intensity b) push these changes also to Bioconductor and ensure that everything is in sync again.

lgatto commented 3 years ago

Not sure why they were removed to be honest. Sorry if I did that, must have happened by mistake.

jorainer commented 3 years ago

No worries - I re-added them. I will then push also to Bioc once all tests passed

lgatto commented 3 years ago

Thanks!