[Bioc-devel] RMassBank build error

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)

jo

On 7 Oct 2016, at 20:14, Rainer Johannes <Johannes.Rainer at 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.Schymanski at 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

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.Rainer at 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.Schymanski at 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

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

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.Rainer at 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.Schymanski at 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

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.Rainer at 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.Schymanski at 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

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)

--
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/

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?

-----Original Message-----
From: Laurent Gatto [mailto:lg390 at cam.ac.uk]
Sent: Sonntag, 9. Oktober 2016 22:03
To: Stravs, Michael
Cc: Rainer Johannes; Schymanski, Emma; Martin Morgan; sneumann at ipb-
halle.de; erik.mueller at ufz.de; Martin Morgan
Subject: Re: [Bioc-devel] RMassBank build error


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/SpectrumMeth
ods.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.Rainer at 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.Schymanski at 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

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)

--
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/

On 10 Oct 2016, at 02:07, Kasper Daniel Hansen <kasperdanielhansen at 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 <lg390 at 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.Rainer at 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.Schymanski at 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

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)

--
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/

	[[alternative HTML version deleted]]

Hi Laurent,

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.

The method did work with MSnbase until the recent constructors
switch... The whole point of the method was to make a copy constructor
possible by explicitely assigning the arguments ("Versioned" has a
problem), otherwise it would do something strange.

But yes, I don't think it will solve 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.

But that's weird as is, no? I thought your constructor did only
relatively mundane stuff, so if this causes problems then it shouldn't
be MSnbase's responsibility to fix them and they are alarming for
every R programmer solution out there...

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?

A pull request to github.com/MassBank/RMassBank would be best I
think. Even though the version on there is possibly newer than the one
on Bioc.

I am currently really busy with urgent stuff and therefore RMassBank
development is on the far-backburner but there's a ton of things I'd
like to implement. Is there a summary of MSnbase improvements that I
could look at, since there were apparently quite many? Maybe there's
other things we could benefit from.

Also, will there be a problem with regards to serialization? We relied
quite heavily on load()ing and save()ing large collections of
RMassBank objects including MSnbase spectra; is there much I will need
to consider in the upgrade path when loading?

Thanks and best wishes,
-Michele

-----Original Message-----
From: Laurent Gatto [mailto:lg390 at cam.ac.uk]
Sent: Sonntag, 9. Oktober 2016 22:03
To: Stravs, Michael
Cc: Rainer Johannes; Schymanski, Emma; Martin Morgan; sneumann at ipb-
halle.de; erik.mueller at ufz.de; Martin Morgan
Subject: Re: [Bioc-devel] RMassBank build error


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/SpectrumMeth
ods.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.Rainer at 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.Schymanski at 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

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)

--
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/