dankelley / oce

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

Object 'found.scan' not found when reading a seabird ctd file #922

Closed angeloSdP closed 8 years ago

angeloSdP commented 8 years ago

I'm new with oce and I have had troubles when reading this file: https://dl.dropboxusercontent.com/u/7610774/Datos/dBdE_1-7.cnv. It is a Seabird ctd file, and when I use:

read.oce("dBdE_1-7.cnv")

I get the following error:

Error in read.ctd.sbe(file, processingLog = processingLog, ...) : object 'found.scan' not found

I have traced this error and notice than it is thrown by the read.ctd.sbe() function. It assigns to the variable found.scan the value TRUE if the expression "scan" is found when reading the file lines. But if this expression is not found, the variable found.scan is not even created. As a consequence, later when the function tries to execute the line:

if (!found.scan)

R throws an error as this variable does not exist. I have been able to manage this problem and read the data by simply adding the line:

found.scan <- FALSE

in the part of the function where variables are initialized (in particular, after the linefound.time<-FALSE).

I'm not really sure if this is a bug of your function, or if it is that my attached file lacks the "scan number" at any place (this is my first contact with ctd files).

Thanks,

richardsc commented 8 years ago

Hi @angeloSdP . Thanks for the very clear issue report and the detective work!

I'm just having a look at the file now, but I think you're correct there is a bug in how oce is handling the found.scan variable. I'll update here when I know more.

In the meantime, are you able to build packages from source? Once the bug is fixed the easiest way to install the fixed package is by following Number 3 at:

http://dankelley.github.io/oce/installation.html

You may need to install some extra tools, depending on your system. For more details, see the wiki page at:

https://github.com/dankelley/oce/wiki/Configuring-a-system-to-be-able-to-build-oce-from-source

On Windows it's pretty easy but OSX can be a bit more involved.

richardsc commented 8 years ago

FYI I've started some code at oce-issues/922

richardsc commented 8 years ago

As I thought, your fix of adding found.scan <- FALSE was the right one. I've created a pull request #923, and will merge to the develop branch shortly.

@dankelley , given that we've run into this initialization problem a couple of times recently, I propose we do a brief code review of this part of read.ctd.sbe to make sure we're not missing any other arguments for initialization.

I plan to look through the code this afternoon. Unfortunately, the devtools "code diagnostics" doesn't catch these things ... I think it is something about block nesting, because I was pretty sure that it does catch things, sometimes. (It certainly catches unused variables, but of course they never hurt anything.)

angeloSdP commented 8 years ago

Thank you for your quick response!! Once sure that the error was simply this, I have no problem in building the package from source and continue working with it. Nice package!

richardsc commented 8 years ago

You're welcome @angeloSdP ! I've merged the PR. You can now build the develop version from source.

If you find that the issue is addressed, go ahead and close the issue. We like to let the issue reporters close to make sure that we've addressed everything.

angeloSdP commented 8 years ago

Perfect! I have loaded the rebuilded version and now the data reading works like a charm. Thanks

dankelley commented 8 years ago

FYI, I went through the code (in a cursory manner ... R/ctd.R is nearly 5000 loc) to look for other problems with scan. I did not see any, but made a tiny tweak as follows (because there was no need to check whether scan was missing after the first test).

 git diff HEAD^1
diff --git a/R/ctd.R b/R/ctd.R
index a4f7ac8..2146a95 100644
--- a/R/ctd.R
+++ b/R/ctd.R
@@ -898,12 +898,12 @@ as.ctd <- function(salinity, temperature=NULL, pressure=NULL, conductivity=NULL,
             stop("lengths of salinity and pressure must match, but they are ", nS, " and ", np)
         if (missing(scan))
             scan <- as.numeric(seq_along(salinity))
-        data <- list(salinity=salinity,
+        data <- list(scan=scan,
+                     salinity=salinity,
                      temperature=temperature,
                      pressure=pressure,
                      sigmaTheta=swSigmaTheta(salinity, temperature, pressure)) # FIXME: what about gsw?
         res@metadata$units$sigmaTheta <- list(unit=expression(kg/m^3), scale="")
-        if (!missing(scan)) data$scan <- as.vector(scan)
         if (!missing(conductivity)) data$conductivity <- as.vector(conductivity)
         if (!missing(quality)) data$quality <- quality
         if (!missing(oxygen)) data$oxygen <- oxygen