Tests fail with latest RNetCDF
mjwoods opened this issue · 7 comments
When testing reverse dependencies of the latest RNetCDF release candidate (https://github.com/mjwoods/RNetCDF/releases/tag/v2.0-2), I found that the current CRAN version of tidync fails R CMD check. The checks also fail at the latest commit 540d6e7 of tidync, and the output log file testthat.Rout.fail contains the following error messages:
> library(testthat)
> library(tidync)
>
> test_check("tidync")
── 1. Failure: utils work (@test-utils.R#51) ──────────────────────────────────
`hv` not equal to hyper_vars(tnc).
Incompatible type for column `id`: x numeric, y integer
Incompatible type for column `ndims`: x numeric, y integer
Incompatible type for column `natts`: x numeric, y integer
── 2. Failure: utils work (@test-utils.R#52) ──────────────────────────────────
`hd` not equal to hyper_dims(tnc).
Incompatible type for column `id`: x numeric, y integer
══ testthat results ═══════════════════════════════════════════════════════════
[ OK: 75 | SKIPPED: 5 | WARNINGS: 0 | FAILED: 2 ]
1. Failure: utils work (@test-utils.R#51)
2. Failure: utils work (@test-utils.R#52)
Error: testthat unit tests failed
Execution haltedThese errors occur due to changes in the data types returned by var.inq.nc, which more closely match the types returned by the netcdf C library.
Oh dang sorry thought we had fixed that - but it is only testing expectations, leads me to to avoid those tests on CRAN in future
Sorry for the inconvenience, Michael. I regret changing the data types now, but it happened a long time ago and seemed like a sensible decision at the time. It does have the benefit of avoiding unnecessary type conversions between the NetCDF C library and R code, and the types are unlikely to change again without a major change to the NetCDF API (and none is planned, AFAIK).
It seems like a good idea to test the results from var.inq.nc in tidync. Perhaps you could coerce them to an expected type, so that the returned type does not matter. That way, the tests would pass for old and new versions of RNetCDF.
Would you mind if I submit the new RNetCDF package now? Or should I delay until you can submit an update of tidync?
No I don't mind, these revdep relationships always confuse me but thinking about it I realised there's no need to delay you - not at all, you should go ahead!
Thanks Michael, I have submitted the new RNetCDF to CRAN.
Hi Michael, is the change of data type in var.inq.nc likely to cause problems for users of tidync? That is, does it affect functionality of the package, apart from a minor failure in R CMD check?
It won't affect users, it's only my tests. You are making integer types where before we had double yeah? That's a good thing for sure! My tests would be better done with equivalence, it was too strict to test on exact type.
I'm pretty confident this will raise an error on CRAN, but it will be an easy fix with a few weeks grace and I'll take a closer look then. Good luck with CRAN!
Here it is, had meant to get onto this earlier - sorry CRAN ...
Please correct before 2019-10-25
https://cran.r-project.org/web/checks/check_results_tidync.html