search

benjcunningham / googlecalendar

Easily access and modify calendars, events, and UI settings via the Google Calendar API
Other
12 stars 6 forks source link

Bad tibble column references #12

Closed benjcunningham closed 8 years ago

benjcunningham commented 8 years ago

Here's the error from the failed Build #41:

Last 13 lines of output:
  'help.start()' for an HTML browser interface to help.
  Type 'q()' to quit R.

  > library(testthat)
  > library(googlecalendar)
  > 
  > test_check("googlecalendar")
  Auto-refreshing stale OAuth token.
  Auto-refreshing stale OAuth token.
  Error: Unknown column 'cid'
  testthat results ================================================================
  OK: 28 SKIPPED: 0 FAILED: 0
  Execution halted

Can't reproduce outside of Travis, but here are some notes from ad1bd66:

Travis builds seemed to be failing because (at least, I suspect) tibbles were being passed to as.body(). Strangely, nothing fails interactively, but it looks like when we'd call df$xxx where xxx does not exist as a column, we get an error rather than the expected NULL (the error is to be expected with the strictness of tibbles).

I'm unsure what exactly causes the tibble issue, so this removes the readr suggest (only being used in testing), replaces it with a regular read.csv(), and changes the as.body dispatch to a regular data.frame.

Not entirely sure this will work, but more annoyingly, I can't reproduce the errors outside of a Travis build...

Looks like the entire test schedule fails out as soon as it hits the as.body() call inside of gc_event_import(). I still think this is an issue with tibbles, and now realize that purrr::by_row() probably coerces data frames too, so the edits in ad1bd66 didn't cover everything.

Still, why does this only happen in Travis? :angry:

benjcunningham commented 8 years ago

Still, why does this only happen in Travis?

Oops. Didn't realize my box hadn't updated to dplyr 0.5.0 yet, which apparently makes the hard switch to erroring on non-existent columns. Hopefully now I can cut down on all the needless commits / reverts / builds while I debug.

benjcunningham commented 8 years ago

Finally got to the root of the problem in 634caad. Based on some of the issues / conversations currently happening in the dplyr, tibble, and purrr threads, it seems that the best fix for now is just using unclass() inside of as.body() to make sure we can still take advantage of df$xyz returning NULL when xyz doesn't exist in the object.

benjcunningham commented 8 years ago

Need to take a look at how the depends are currently aligned between all of the packages, but with tibble 1.1, it looks like this might be able to be re-rewritten after all. Probably low priority, though.