Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On disk vector #62

Open
wants to merge 11 commits into
base: OnDiskVector
Choose a base branch
from
Open

On disk vector #62

wants to merge 11 commits into from

Conversation

dsammour
Copy link

Hi Sebastian,

This a pull request based on OnDiskVector class. I have the following comments:

  • while I find tracking changes to the OnDiskVector a good idea, I think the following check defeats its purpose (it is commented-out in this pull request):
 .isModified.OnDiskVector <- function(x) {
     m <- readBin(x@mpath, integer(), n=1L, size=NA_integer_, endian="little")
     if (m != x@modification)
        stop(x@path, " was modified by a different object.")
     FALSE
 }

I work solely on imzML files, there you'll inevitably need to modify the OnDiskVector (mass and/or intensity) via assignments such as:

s <- calibrateIntensity(s) # s is a list of MassSpectraOnDisk

Whenever this is called, the above check will through an error. To circumvent this, we will need to write separate methods for MassSpectraOnDisk where the following call would be enough:

calibrateIntensity(s) # s is a list of MassSpectraOnDisk
  • in .transformIntensity, wouldn't the following part violate the mass and intensity assignments ?
#.transformIntensity
if (na.rm) {
      naIdx <- which(!is.na(object@intensity))
      object@intensity <- object@intensity[naIdx]
      object@mass <- object@mass[naIdx]
}

namely:

#intensity-methods
 if (length(object@intensity) == length(value)) {
    object@intensity <- as.double(value)
  } else {
    stop("Lengths of intensity(", length(object@intensity),
         ") and value (", length(value), ") have to be equal.")
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants