rformassspectrometry / MsIO

Serializing/importing mass spectrometry data objects eventually using language-agnostic formats.
https://rformassspectrometry.github.io/MsIO
2 stars 0 forks source link

Move import/export methods for MsBackendMzR from xcms #2

Closed jorainer closed 4 months ago

jorainer commented 5 months ago

Move the functionality to serialize a MsBackendMzR object to a text file from xcms (in this PR).

The key concept/question will be if we can add S4 methods for e.g. the MsBackendMzR class without importing the Spectra package.

jorainer commented 5 months ago

@philouail , can you please move the code over from xcms? I would put the parameter object definition and functions into a PlainTextParam.R file (or FlatFileParam.R file?) and then add a method for MsBackendMzR in a MsBackendMzR.R file. Let's start with that and then successively add methods for Spectra, MsExperiment etc on top (all in their .R files). For the documentation, that will be a bit tricky maybe how to put that, but I think it would make sense to have for each class (e.g. MsBackendMzR a documentation, and in that put the individual ones for the parameters (e.g. saveMsObject,MsBackendMzR,PlainTextParam etc ).

We can obviously think of an other way to structure the files and documentation.

philouail commented 5 months ago
philouail commented 5 months ago

Little addition in term of to-do steps ( and after discussing with @lgatto) after moving everything, we should discuss what can be changed as to use the alabaster package. Here i'm talking about the PlainTextParam code ( or then it could be renamed somehow if we only use Json export maybe we should indicate that). The package is sturdy and there are validation of object (we then don't need to worry too much about that). And we shouldn't try to reinvent what exist. Also would motivate using this for the summarizedExperiment object import/export.

I will look a bit more into the package and test the functions ! :)

philouail commented 5 months ago

The key concept/question will be if we can add S4 methods for e.g. the MsBackendMzR class without importing the Spectra package.

Do you not want to be dependent on Spectra ?

jorainer commented 5 months ago

Do you not want to be dependent on Spectra ?

no. ideally, this package should not depend on Spectra or MsExperiment et al. If a user wants to export or import e.g. a MsBackendMzR or similar object he/she will anyway have loaded the Spectra package before. so, no need to make this package dependent on the other.

jorainer commented 5 months ago

Re alabaster: agree that we should not reinvent stuff. but we need also to ensure that the code provides functionality we want/need. I need to first have a close look at alabaster... then we discuss.

philouail commented 4 months ago

Do you not want to be dependent on Spectra ?

no. ideally, this package should not depend on Spectra or MsExperiment et al. If a user wants to export or import e.g. a MsBackendMzR or similar object he/she will anyway have loaded the Spectra package before. so, no need to make this package dependent on the other.

Mmmm okay so I think we need to discuss more because @lgatto saw it the other way around, with putting the MsIO package as "enhance" in e.g. the Spectra package, but MsIO by itself is dependent on these packages. (If I understood well). I also am not sure how not to make it dependent on spectra as I am using functionalities of the Spectra package. So we can discuss that together next week :)

and yes alabaster on paper I believe it is very nice and basically do what we do by serializing (even though i haven't managed to make it work with our objects...)

philouail commented 4 months ago

@jorainer https://www.bioconductor.org/packages/release/bioc/html/alabaster.base.html