lgatto / Spectrum

Spectrum Infrastructure for Mass Spectrometry Data
2 stars 1 forks source link

New Spectrum class #2

Open lgatto opened 5 years ago

lgatto commented 5 years ago

The new Spectrum class is essentially a list with a set of predefined (mandatory) elements. It can be extended on the fly with optional elements. The fact that it is a simple list actually eliminates the need for specific accessors and setters, as we can simply used $ and $<-. I find it quite elegant.

Comments and suggestions welcome.

Cc @jorainer @meowcat

jorainer commented 5 years ago

We might however want to overwrite the $<- method to ensure that the the replacement value is of the correct type - eventually by simply calling validObject which checks that each of the mandatory elements has the correct data type (and length 1).

lgatto commented 5 years ago

This is handled by to validity methods (still need to add checks on cardinality though).

jorainer commented 5 years ago

But do we want to completely drop the acessors? I thought that might be a nice way to define the interface (that would also be backward compatible).

lgatto commented 5 years ago

I don't know, future will tell. But if we want accessors, we need to stop $ from working, otherwise I foresee users using the latter only. Which then suggests that may be they are a good option.

jorainer commented 5 years ago

OK, let's stick for now with the $. We just have to sync then also development in Spectra accordingly.

lgatto commented 5 years ago

How does this impact Spectra? What am I missing?

jorainer commented 5 years ago

I thought we might want to have the same way/interface to access spectra variables from both Spectrum and Spectra objects.

lgatto commented 5 years ago

OK, I see. Let's not change plans in Spectra. If we want the same interface (which indeed makes sense), we'll add in to Spectrum objects.

meowcat commented 5 years ago

I'm a big fan of this:

mz <- as.numeric(sample(1000, 10))
i <- as.numeric(sample(1e3:1e5, 10))
sp[c("intensity", "mz")] <- list(i, mz)