Remove Ramp backend with dependencies

sneumann / mzR

This is the git repository matching the Bioconductor package mzR: parser for netCDF, mzXML, mzData and mzML files (mass spectrometry data)

42 stars 28 forks source link

Remove Ramp backend with dependencies #251

Closed jorainer closed 3 years ago

jorainer commented 3 years ago

This PR:

removes the Ramp Rcpp module and all related R code
removes the Ramp C++ code and all other (required?) libraries (i.e. most files in the base src/ folder)
remove and update unit tests.

lgatto commented 3 years ago

I suppose we are sure we won't never need it again? I personally doubt so, but just checking.

sneumann commented 3 years ago

I would suggest to pull this into main only after the BioC release, leaving time to check for regressions.

sneumann commented 3 years ago

A regression I currently see is that we lose ability to read mzData, which will e.g. require some files in msdata and all of mtbls2 to be converted to mzML. But then it is a decade after mzML was published and recommended. Yours, Steffen

jorainer commented 3 years ago

If we really need mzData we could have a separate mzRamp package that contains the Ramp-based backend and provides that to mzR. But since that would involve quite some hacking I would only do that if absolutely necessary...

lgatto commented 3 years ago

@jorainer - what do we gain by removing the mzRamp backend? Is the compilation time reduced?

sneumann commented 3 years ago

We currently have two ramp.c implementations, the original one from xcms in 200X and the one embedded in pwiz. It was me asking Jo to take the old one out, since that was causing so much pain and failure updating to a newer pwiz. A newer pwiz should be priority now. Compile times are unfortunately not reduced much. Yours, Steffen

lgatto commented 3 years ago

Ah, thanks for clarifying the reason behind this. I totally agree that updating pwiz should be the priority.