dankelley / oce

R package for oceanographic processing
http://dankelley.github.io/oce/
GNU General Public License v3.0
141 stars 42 forks source link

post ctdTrim() error, argument "j" missing #432

Closed annidjurhuus closed 10 years ago

annidjurhuus commented 10 years ago

I have had problems using the ctdTrim() function with oce version 0.9-13. When trying to plot the trimmed ctd I get an error saying that argument "j" is missing. The plots that show up are the two first plots, but the second two are missing. After the ctdTrim() it is possible to use the ctdDecimate() and all the data seems to be present, but not available to use, e.g. for binning.

The code

ctd <- read.ctd("ctd")
ctdtrim <- ctdTrim(ctd, "range",parameters=list(item="pressure", from=12))
ctdtrim <- ctdTrim(ctdtrim, method='index', parameters=1:which.max(pressure(ctdtrim)))
plot(ctdtrim)

yields error:

Error message:
Error in as(x, "oce")[[i, j, drop]] : 
  error in evaluating the argument 'j' in selecting a method for function '[[': Error: argument "j" is missing, with no default
dankelley commented 10 years ago

I think this works in the develop branch. See test code at oce-issues

dankelley commented 10 years ago

PS. if you are reporting more on the bug, and get an error, please use

traceback()

so as to provide a hint as to where the error arose.

annidjurhuus commented 10 years ago

The issue is still not addressed with the development package. I downloaded the package from the link http://dankelley.github.io/oce/.

library(devtools)
install_github("oce", "dankelley", "develop")
install_github("ocedata", "dankelley", "master", build_vignettes=FALSE)

I didn't have latex installed, so I suppressed the creation of vignettes, or else the package wouldn't install. Initially this didn't work because the newest XCode version (5.1.1) doesn't have command line tools automatically installed. Then I had to force the Makeconf file to use clang instead of llvm, making the following changes to the file:

CC=clang
CXX=clang++

I have now restarted my system and tried the same code as originally, but it still doesn't work and I get the exact same errors.

dankelley commented 10 years ago

Hm. I'm actually at a loss here. I also have an OSX machine, and installed the Xcode command line tools and found no need to change CC etc.

In any case, I do not know how to proceed because I cannot reproduce this error.

I wonder if a smaller CNV file could be made, and posted somewhere public, so that others could try to reproduce the error? (There are other OSX users who use CNV files and oce.)

Sorry.

richardsc commented 10 years ago

Sorry, wasn't looking closely and left a comment that didn't make any sense.

Maybe you need to do:

library(devtools)
install_github("oce", "dankelley", "develop", build_vignettes=FALSE)
install_github("ocedata", "dankelley", "master", build_vignettes=FALSE)

to keep the develop version from trying to build vignettes?

I am also on OSX (10.8) with XCode 5.1.1 and I have no problem building the develop branch.

richardsc commented 10 years ago

In fact, on my system, this line:

install_github("ocedata", "dankelley", "master", build_vignettes=FALSE)

fails because the ocedata package has no vignettes. So maybe try:

library(devtools)
install_github("oce", "dankelley", "develop", build_vignettes=FALSE)
install_github("ocedata", "dankelley", "master")

If that still doesn't work I'm sure looking at the CNV file will let us figure out what's going on.

dankelley commented 10 years ago

Another thing for @annidjurhuus to try: instead of using the (private) data, try working with the dataset that's provided with oce,

ctd <- read.oce(system.file("extdata", "ctd.cnv", package="oce"))

and then the rest of the tests.

What I'm hoping to do is to find a way to get a reproducible bug.

annidjurhuus commented 10 years ago

It seems the only way it works to install the development package is without vignettes, I have not installed latex on my computer, which I think is a prerequisite to compile the vignettes?

I tried installing the development packages the way you suggested @richardsc but that is not working either and I still get the same error with argument "j" missing. There are no error messages during the installation, I have looked very closely.

@dankelley , the reason I changed the CC to clang was to force it to get make, which was not found when I first tried to installed it.

I just tried using the data @dankelley supplied as well, and that does not work either with same error again. The strange thing is that I have tried with different operating systems and different data (Not using the development package, yet), which yields the same error. I get the error with any plotting function and when using binAverage, the ctd object is not selectable. I cannot select within it as I would do with a list, which was possible with the older version of 'oce'.

annidjurhuus commented 10 years ago

When using plotScan on ctd object (data from @dankelley) then

ctd <- read.oce(system.file("extdata", "ctd.cnv", package="oce"))
plotScan(ctd)

yields this error

Error in plot(x[[name]], x@data$pressure, xlab = name, ylab = resizableLabel("p",  : 
  error in evaluating the argument 'x' in selecting a method for function 'plot': Error in as(x, "oce")[[i, j, drop]] : 
  error in evaluating the argument 'j' in selecting a method for function '[[': Error: argument "j" is missing, with no default

and then

traceback()

yields

2: plot(x[[name]], x@data$pressure, xlab = name, ylab = resizableLabel("p", 
       "y"), yaxs = "r", type = type)
1: plotScan(ctd)
dankelley commented 10 years ago

DK about to go offline ... it would help if you could source() the file oce/R/ctd.R and then do the traceback, to get a line number that could be posted here. Then maybe you could post part of that ctd.R file (say 10 lines above and below), which might tell if we are looking at different files. I'm sort of wondering whether your install worked. Maybe @richardsc has other ideas.

richardsc commented 10 years ago

I am unable to reproduce the error by doing

ctd <- read.oce(system.file("extdata", "ctd.cnv", package="oce"))
plotScan(ctd)

using oce version 0.9-13 (installed from CRAN). Dan's suggestion is good, but first it might be a good idea to see what you're system is actually running. After doing library(oce), run

R.version
sessionInfo()

at the R console and paste the output here.

annidjurhuus commented 10 years ago

ok. I am about to go offline as well. I will try installing it again and look through what is actually happening as well and post it here later. @richardsc I was using version 0.9-13 before, which I installed the normal way through R but it never worked, bare in mind that I have used the 'oce' package before on the same computer and it worked fine until I installed version 0.9-13.

Thank you both very much for helping me with this problem.

richardsc commented 10 years ago

Just poking around in the code at oce/R/ctd.R and I think that the line that is responsible for throwing the error:

Error in plot(x[[name]], x@data$pressure, xlab = name, ylab = resizableLabel("p",  : 
  error in evaluating the argument 'x' in selecting a method for function 'plot': Error in as(x, "oce")[[i, j, drop]] : 
  error in evaluating the argument 'j' in selecting a method for function '[[': Error: argument "j" is missing, with no default

is most likely line 99

97             } else {
98                 ## I use 'as' because I could not figure out callNextMethod() etc
99                  rval <- as(x, "oce")[[i, j, drop]]
100                  if (is.null(rval))
101                     stop("in ctd[[\"", i, "\"]]: no such item", call.=FALSE)
102                  rval
103              }

which indicates that for some reason the [[ method isn't functioning properly for you. This is an odd error, and my best guess (at least until I see the output from sessionInfo()) is that it must be something specific to your system.

annidjurhuus commented 10 years ago

ok. Running plot(), plotScan() or binAverage() doesn't give you any errors? not even with the 0.9-13 version on a mac?

richardsc commented 10 years ago

No, no errors for plot(), or plotScan(). I'm not sure what you mean by binAverage() since that is not a function designed to take a ctd object as input.

library(oce)
sessionInfo()
ctd <- read.oce(system.file("extdata", "ctd.cnv", package="oce"))
plot(ctd)
plotScan(ctd)

gives:

> sessionInfo()
R version 3.1.0 (2014-04-10)
Platform: x86_64-apple-darwin10.8.0 (64-bit)

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] oce_0.9-13    mapproj_1.2-2 maps_2.3-6   

loaded via a namespace (and not attached):
[1] tools_3.1.0
> ctd <- read.oce(system.file("extdata", "ctd.cnv", package="oce"))
> plot(ctd)
> plotScan(ctd)

i.e. no errors for plot() or plotScan(). Note oce version 0.9-13 which is the CRAN version of oce, installed with install.packages('oce'), and not from the develop branch.

binAverage(ctd) gives this error:

Error in binAverage(ctd) : must supply 'y'

but that has nothing to do with the original issue, as binAverage() is not designed to work on a ctd object like that. If you want to average CTD data into bins (e.g. pressure), I think you want to use ctdDecimate().

richardsc commented 10 years ago

What version of R are you using? Note from my sessionInfo() I'm using 3.1.0, but I've tried to reproduce your error on 3.0.3 and 2.15.3 and the sample code block from above works fine on all of them.

annidjurhuus commented 10 years ago

I am using R version 3.0.2. I have figured out what causes the problem. Unlike the previous version the current oce version creates the fourth map plot with the location of the CTD cast. There is an interference between the oce package and the maptools package. It does not work even after detaching the maptools package but only starting on a clean workspace when using the oce package. I will try to figure out the code causing the problem by looking through this again as @dankelley suggested. Will get back to you on this issue.

dankelley commented 10 years ago

Thanks. We now have a reproducible bug:

library(oce)
library(maptools)
data(ctd)
plot(ctd)

yields

Error in as(x, "oce")[[i, j, drop]] : 
  error in evaluating the argument 'j' in selecting a method for function '[[': Error: argument "j" is missing, with no default

and I will now look into this.

dankelley commented 10 years ago

It's nothing to do with the map-drawing part of plot(ctd); rather, the problem is manifested in e.g.

ctd[["temperature"]]

and thus the crux of the matter is that the maptools package seems to have over-ridden the oce [[ accessor operator.

annidjurhuus commented 10 years ago

ok, good to know, thank you very much for your help!

dankelley commented 10 years ago

I am not sure there will be a result because it may involve a bug in maptools. I've been reading the maptools code for a half hour and am not seeing where it over-loads the definition of [[. However, clearly it does do something to this, as illustrated below, from test 432B.R

> showMethods('[[')
Function: [[ (package base)

> library(oce)
Loading required package: mapproj
Loading required package: maps
Loading required package: ncdf4
Loading required package: tiff
> showMethods('[[')
Function: [[ (package base)
x="adp"
x="adv"
x="ctd"
x="echosounder"
x="gps"
x="landsat"
x="oce"
x="sealevel"
x="section"
x="tidem"
x="topo"
x="windrose"

> library(maptools)
Loading required package: sp
Checking rgeos availability: FALSE
    Note: when rgeos is not available, polygon geometry     computations in maptools depend on gpclib,
    which has a restricted licence. It is disabled by default;
    to enable gpclib, type gpclibPermit()
> showMethods('[[')
Function: [[ (package base)
x="adp", i="ANY", j="ANY"
x="adv", i="ANY", j="ANY"
x="ctd", i="ANY", j="ANY"
x="echosounder", i="ANY", j="ANY"
x="gps", i="ANY", j="ANY"
x="landsat", i="ANY", j="ANY"
x="oce", i="ANY", j="ANY"
x="sealevel", i="ANY", j="ANY"
x="section", i="ANY", j="ANY"
x="Spatial", i="ANY", j="missing"
x="tidem", i="ANY", j="ANY"
x="topo", i="ANY", j="ANY"
x="windrose", i="ANY", j="ANY"
richardsc commented 10 years ago

Perhaps you should check the sp package, which map tools depends on. Note the "Spatial" object that is added to the showMethods output.

dankelley commented 10 years ago

Yes, doing that now actually. Also looking at the (somewhat confusing) docs on S4 objects, just in case oce has done something wrong that lets sp (or whatever) ruin its definitions of the accessor. No luck so far.

I have slit a new branch named issue432 in which I'll do tests. This is the sort of bug that can be hard to find because it possibly requires understanding not just one's own code, but also code written by others. And of course if the issue is that the other code is problematic, then there's no way to fix the issue alone.

annidjurhuus commented 10 years ago

Just a note on the previous from @richardsc not sure if it makes any difference, but it definitely seems to be the packages (i.e. 'raster', 'rastervis', 'rgdal', 'rsamtools', 'rtracklayer') that depend on 'sp' that make 'oce' fail. However, the 'phyloseq' package masks distance from the 'oce' package but doesn't require 'sp', it gives me the same error as before if I have phyloseq loaded.

library('phyloseq') library("phyloseq", lib.loc="/Library/Frameworks/R.framework/Versions/3.0/Resources/library")

Attaching package: ‘phyloseq’

The following object is masked from ‘package:oce’:

distance

Raster: library("raster", lib.loc="/Library/Frameworks/R.framework/Versions/3.0/Resources/library") Loading required package: sp

Attaching package: ‘raster’

The following objects are masked from ‘package:oce’:

distance, extract
dankelley commented 10 years ago

I am close to a solution but will want to test it for a while and so it might not get into github for a few days. Now that I can reproduce the error, the work should be relatively quick.

PS. I have asked github.com if there is a way to provide instructions on the 'submit new issue' page; it will request the use of sessionInfo(), which would have revealed the package interaction problem from the start.

annidjurhuus commented 10 years ago

I am sorry about that. I am not so experienced using github, which I guess is obvious. My apologies.

dankelley commented 10 years ago

@annidjurhuus -- no worries. This is why I want to get the text onto the github webpage, to make it easier for everyone. Thank you for reporting the bug, and the probable source!! Although users may think that developers are doing them a favour, it's quite the reverse. If I had run into this bug during my own work, I'd be very frustrated in having to delay my work to fix the bug. Your timing was perfect ... fixing the bug was just something to spend some weekend time on.

annidjurhuus commented 10 years ago

I am glad I could keep your weekends occupied @dankelley , anytime. Now the problem has been found I can get back to my work as well. Thank you again. I will post any problems I might find in the future.

dankelley commented 10 years ago

I think this works now in the develop branch. @annidjurhuus please test and report back here, and if the fix seems OK, please close the issue.

annidjurhuus commented 10 years ago

Yes, it works perfectly well now. Thank you.