I followed Kasper's suggestion and removed the new("Versioned") calls from the prototype of the Spectrum, Spectrum1 and Spectrum2 classes and added the corresponding calls (e.g. classVersion(.Object)["Spectrum1"] <- "0.2.0") to the objects' initialize methods. With that setup I don't get (thus far) the random errors in MSnbase.
I would suggest to update the help page of "Versioned", as there the usage of Versioned was described exactly as we did in MSnbase. cheers, jo > On 10 Oct 2016, at 02:07, Kasper Daniel Hansen <kasperdanielhan...@gmail.com> > wrote: > > You (and everyone else) should not construct new instances by using new() > together with prototype. It is true that we did that long time ago in > Biobase, but I think the collective lesson is that it is not really > flexible/stable enough (ok, bad wording, but it is Sunday night). What we > recommend now is to create a constructor function, usually the class name, > like GRanges(). Of course, inside the constructor you would then call > new(), which works ok for simple cases. > > On Sun, Oct 9, 2016 at 4:02 PM, Laurent Gatto <lg...@cam.ac.uk> wrote: > >> >> Dear Michael and RMassBank developers, >> >> On 9 October 2016 15:53, Stravs, Michael wrote: >> >>> Hi all, >>> >>> I now have all packages installed to test things and can reproduce the >>> problem. I must confess that I don't have a deep understanding on how S4 >>> initialization works, so solution attempts on my side are very >>> trial-and-errory. >>> >>> I will have to look into the workings of these initialization methods >>> anyway in the future. The RMassBank version currently on Bioc-devel uses >>> the default constructor (i.e. doesn't actually define an "initialize" >>> method). >>> >>> However, one of my development versions has an initialize method, which >>> I introduced to work around a bug with Versioned initialize (see my >>> question here [1], the reply [2], and my current code [3]). That one is >>> *also* incompatible with the new MSnbase constructors because of some >>> argument mapping issues (but there might also be a workaround for that.) >>> >>> [1] https://stat.ethz.ch/pipermail/bioc-devel/2016-January/008528.html >>> [2] https://stat.ethz.ch/pipermail/bioc-devel/2016-January/008576.html >>> [3] >>> https://github.com/MassBank/RMassBank/blob/s4power/R/ >> SpectrumMethods.R#L124 >> >> I don't think that your initialize method will work as it is, because it >> passess arguments to callNextMethod, which doesn't have them. But, to be >> honest, I don't think that any intialize method will sort the current >> problem. >> >> I am not sure how to best handle to problem. To sort your problem, we >> (the MSnbase maintainers) could revert to our old initialize,Spectrum >> method, but some very weird bugs (in R? in some very low-level C code?) >> result in random crashes. We are looking for something else. >> >> Anyway, if we don't manage to find an acceptable solution for MSnbase, I >> will submit an RMassBank patch that will sort the error (and other >> things resulting from MSnbase improvements) out. What would be the best >> way to submit a patch - email, a pull request to a github repo? >> >> Best wishes, >> >> Laurent >> >>> On 07.10.2016 22:34, Laurent Gatto wrote: >>>> On 7 October 2016 21:30, Rainer Johannes wrote: >>>> >>>>> an update: >>>>> >>>>> I've switched back to the "default" initialize methods for Spectrum1 >>>>> and Spectrum2 objects in MSnbase (not implemented in C) and with these >>>>> installing RMassBank seems to work. >>>>> I have however now to run some intense torture tests on MSnbase to >>>>> ensure that we don't run again into the random memory problems that we >>>>> had in MSnbase (issue https://github.com/lgatto/MSnbase/issues/138) >>>> I have done the same thing, which also needed addressing some unit >>>> tests. I will push these updates to master for after a final package >>>> check, but wait for the results of your intense torture tests before >>>> committing to svn. >>>> >>>> Laurent >>>> >>>>> jo >>>>> >>>>> >>>>>> On 7 Oct 2016, at 20:14, Rainer Johannes <johannes.rai...@eurac.edu> >> wrote: >>>>>> >>>>>> we could try to switch back from the C-constructors to the "old" >> ones, I'll check later >>>>>> >>>>>>> On 7 Oct 2016, at 20:03, Schymanski, Emma <emma.schyman...@eawag.ch> >> wrote: >>>>>>> >>>>>>> Hi Laurent, >>>>>>> >>>>>>> I don't have an answer for you right now - but in CC are also the >> other 3 involved in trying to fix this on our side... >>>>>>> Just to make sure that all have the respective email addresses to >> try speed up the debugging... >>>>>>> >>>>>>> Thanks! >>>>>>> Emma >>>>>>> ________________________________________ >>>>>>> From: Laurent Gatto [lg...@cam.ac.uk] >>>>>>> Sent: Friday, 7 October 2016 7:52 PM >>>>>>> To: Schymanski, Emma; Martin Morgan >>>>>>> Cc: bioc-devel@r-project.org; Rainer Johannes >>>>>>> Subject: Re: [Bioc-devel] RMassBank build error >>>>>>> >>>>>>> Dear Emma, >>>>>>> >>>>>>> The error is very strange indeed, and I hope Martin can help us out >> here. >>>>>>> >>>>>>> With the latest RMassBank and MSnbase, I get >>>>>>> >>>>>>>> new("RmbSpectrum2") >>>>>>> Error in initialize(value, ...) : >>>>>>> 'initialize' method returned an object of class “Spectrum2” instead >> of >>>>>>> the required class “RmbSpectrum2” >>>>>>> >>>>>>> which reproduces the error. >>>>>>> >>>>>>> The RmbSpectrum2 class is defined in a standard way >>>>>>> >>>>>>> .RmbSpectrum2 <- setClass("RmbSpectrum2", >>>>>>> representation = representation( >>>>>>> satellite="logical", >>>>>>> low="logical", >>>>>>> rawOK ="logical", >>>>>>> good = "logical", >>>>>>> mzCalc = "numeric", >>>>>>> formula = "character", >>>>>>> dbe = "numeric", >>>>>>> formulaCount = "integer", >>>>>>> dppm = "numeric", >>>>>>> dppmBest = "numeric", >>>>>>> ok = "logical", >>>>>>> info = "list" >>>>>>> ), >>>>>>> contains=c("Spectrum2"), >>>>>>> prototype = prototype( >>>>>>> satellite = logical(), >>>>>>> low = logical(), >>>>>>> rawOK = logical(), >>>>>>> good = logical(), >>>>>>> mzCalc = numeric(), >>>>>>> formula = character(), >>>>>>> dbe = numeric(), >>>>>>> formulaCount = integer(), >>>>>>> dppm = numeric(), >>>>>>> dppmBest = numeric(), >>>>>>> ok = logical(), >>>>>>> info = list(), >>>>>>> new("Versioned", >> versions=c(classVersion("Spectrum2"), >>>>>>> >> RmbSpectrum2 = "0.1.0")) >>>>>>> )) >>>>>>> >>>>>>> >>>>>>> and >>>>>>> >>>>>>>> new("Spectrum2") >>>>>>> Object of class "Spectrum2" >>>>>>> Precursor: NA >>>>>>> Retention time: : >>>>>>> Charge: NA >>>>>>> MSn level: 2 >>>>>>> Peaks count: 0 >>>>>>> Total ion count: 0 >>>>>>> >>>>>>> works. >>>>>>> >>>>>>> The MSnbase maintainers have had a bit of a struggle with spurious >> and >>>>>>> stange failures in the recent past (see [1]). This ans a whole new >>>>>>> backend in the package have led to the following initialize method, >> that >>>>>>> constructs the class directly in C >>>>>>> >>>>>>> setMethod("initialize", >>>>>>> "Spectrum2", >>>>>>> function(.Object, msLevel = 2L, peaksCount = length(mz), >>>>>>> rt = numeric(), acquisitionNum = NA_integer_, >>>>>>> scanIndex = integer(), tic = 0L, mz = numeric(), >>>>>>> intensity = numeric(), fromFile = numeric(), >>>>>>> centroided = NA, smoothed = NA, >>>>>>> polarity = NA_integer_, merged = 1, >>>>>>> precScanNum = NA_integer_, precursorMz = NA, >>>>>>> precursorIntensity = NA, precursorCharge = >> NA_integer_, >>>>>>> collisionEnergy = NA) { >>>>>>> .Object <- Spectrum2_mz_sorted(msLevel, peaksCount, rt, >>>>>>> acquisitionNum, >> scanIndex, >>>>>>> tic, mz, intensity, >> fromFile, >>>>>>> centroided, smoothed, >> polarity, >>>>>>> merged, precScanNum, >> precursorMz, >>>>>>> precursorIntensity, >> precursorCharge, >>>>>>> collisionEnergy) >>>>>>> if (validObject(.Object)) >>>>>>> .Object >>>>>>> }) >>>>>>> >>>>>>> >>>>>>> Why is calling new("RmbSpectrum2") direclty returning an Spectrum2 >>>>>>> object? Is this due to us not calling callNextMethod? >>>>>>> >>>>>>> Laurent >>>>>>> >>>>>>> [1] https://github.com/lgatto/MSnbase/issues/138 >>>>>>> >>>>>>> >>>>>>> On 7 October 2016 17:02, Schymanski, Emma wrote: >>>>>>> >>>>>>>> Hi BioC team, >>>>>>>> >>>>>>>> In all the mzR troubles, it slipped through that RMassBank had a >> build error of it's own presumably caused by an update to MSnbase - for >> some reason we never received the email with the build error reports?! >>>>>>>> We have discovered the error now, very late, and are onto finding >> out the cause to fix it but it is not straightforward. Now that it is the >> day of the deadline to pass build without error, are we able to have a >> little leeway if needed? It's taken us the whole day to get the right >> binaries to actually have a chance to start fixing... >>>>>>>> Can someone also check or explain why we no longer receive the >> emails reporting errors to us? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Emma (on behalf of the others) >>>>>>>> _______________________________________________ >>>>>>>> Bioc-devel@r-project.org mailing list >>>>>>>> https://stat.ethz.ch/mailman/listinfo/bioc-devel >>>>>>> >>>>>>> -- >>>>>>> Laurent Gatto | @lgatt0 >>>>>>> http://cpu.sysbiol.cam.ac.uk/ >>>>>>> http://lgatto.github.io/ >>>> >> >> >> -- >> Laurent Gatto | @lgatt0 >> http://cpu.sysbiol.cam.ac.uk/ >> http://lgatto.github.io/ >> >> _______________________________________________ >> Bioc-devel@r-project.org mailing list >> https://stat.ethz.ch/mailman/listinfo/bioc-devel >> > > [[alternative HTML version deleted]] > > _______________________________________________ > Bioc-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/bioc-devel _______________________________________________ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel