dankelley / oceglider

R package for processing ocean glider data
https://dankelley.github.io/oceglider/
3 stars 1 forks source link

subset() does not test properly #18

Closed dankelley closed 5 years ago

dankelley commented 5 years ago

Well, this is a head-scratcher. The image shows the problem ("develop" branch commit 9372910d0e3684676ed7472cc1bca01ad3d9e698).

In the left side of the image, you can see the results of doing source() on tests/testthat/test_subset.R, namely that it works. (There is extra debugging stuff there because of my scratched head...)

In the right hand side, you can see that clicking the build/test menu item in Rstudio leads to an error. I don't quite know how this can be. Also, I have looked at similar code and tests in oce, and I think I am doing the same thing. (See e.g. oce/R/argo.R for the "subset" definition, and compare with that in oceanglider/R/oceanglider.R for its "subset" definition.)

I've run into this thing before with oceanglider, which is why I invented gliderTrim. But I really want to remove gliderTrim (which is now hidden in comments) because I think subset is the function that will make most sense to users.

Screen Shot 2019-03-27 at 3 15 29 PM

dankelley commented 5 years ago

I made a mini-package so I could provide it as an example for a query on a discussion group. After some fiddling (a lot faster on my home machine, and with a tiny package), I got it working. So, no need to ask for help ... I'll just need to see what I can do to mimic this in the actual package.

I'm pretty sure the issue relates to what particular #' @ items are needed by roxygen2 for it to create a proper the NAMESPACE file. (It's not about the R code.)

Likely, I'll do this tomorrow, in the "dk" branch first, then merged into the "develop" branch. The changes will probably not involve R/seaexplorer.R at all, because the "subset" method is defined in R/oceanglider.R, along with other basic things.

dankelley commented 5 years ago

This is fixed in "develop" branch commit 1f22b78c63ad10f1a95f1ed2f8b59a22cfdd3718, and if you click on the link you'll see that involved just 1 line of Roxygen. I fiddled with that for hours, for 27 characters. I thought I typed a bit faster than that 😜

dankelley commented 5 years ago

Hm. Maybe I spoke too soon, or maybe the changes I made today have broken this again, but this bug is back.

To be clear, the code at https://www.dropbox.com/sh/pthbhrb3ffppfa8/AAAaolvYngBZ8SrnP6-Xb0OSa?dl=0 works properly, and does the full build/test sequence properly, including the following tests:

library(glidr)
context("subset")

test_that("numeric subset", {
          g <- new("glidr")
          values <- 1:10
          g@data$values <- values
          firstTwo <- subset(g, values < 3)
          keep <- values < 3
          expect_equal(firstTwo@data$values, values[keep])
})

test_that("character subset", {
          g <- new("glidr")
          values <- 1:10
          g@data$values <- values
          first <- subset(g, "one")
          expect_equal(first@data$values, values[1])
})
dankelley commented 5 years ago

Fixed (again, I guess, although I'm not sure I tested it correctly before). The key part of the diff is as follows

-#' @name oceanglider
+#' @name oceanglider-class

A consequence of this is that now when you do library(oceanglider) you will see a warning about over-riding the oce plot method. But that's a good thing.

I'll close this.

Below is the commit entry.

commit 250ee8a3776ae2ddadbc6d9504fa5444483ba05a (HEAD -> develop, origin/develop) Author: dankelley kelley.dan@gmail.com Date: Sat Mar 30 15:18:52 2019 -0300

make subset work