Repository navigation
Address issues 259 260 261 - #264
Conversation
|
📖 pkgdown preview for this PR: https://doi-usgs.github.io/loadflex/preview/pr264/ |
|
Running tests with my temp scripts. r$> source('temp/run-tests.R')
r$> run_tests(phase_completed='Edits for PR #264, "Address issues 259 260 261"')
r$> source('temp/compare-tests.R')
r$> compare_edits(old_file='temp/test-output-261005-1736.rds', new_file='temp/test-output-261013-1102.rds')
OLD: temp/test-output-261005-1736.rds (Phase 2 complete — tidyverse-drift fixes applied)
Tests: 89 | Passed: 41 | Failed: 61 | Errors: 16 | Warnings: 21 | Skipped: 24
NEW: temp/test-output-261013-1102.rds (Edits for PR #264, "Address issues 259 260 261")
Tests: 89 | Passed: 35 | Failed: 61 | Errors: 22 | Warnings: 21 | Skipped: 24
MODE: structured (.rds sidecars found)
========================================================
FAILURES (structured, matched by file + test)
========================================================
Totals: OLD = 61 across 10 tests | NEW = 61 across 10 tests
IMPROVED - FAILURES fully gone from these tests
-----------------------------------------------
(none)
DEGRADED - FAILURES newly present in these tests
------------------------------------------------
(none)
IMPROVED - fewer FAILURES in these tests
----------------------------------------
(none)
DEGRADED - more FAILURES in these tests
---------------------------------------
(none)
========================================================
ERRORS (structured, matched by file + test)
========================================================
Totals: OLD = 16 across 16 tests | NEW = 22 across 22 tests
IMPROVED - ERRORS fully gone from these tests
---------------------------------------------
(none)
DEGRADED - ERRORS newly present in these tests
----------------------------------------------
test-44-plots.R - plotEGRET works for fitdat and meta (now 1)
test-51-EGRET.R - convertToEGRET allows metadata and data or newdata without model (now 1)
test-51-EGRET.R - convertToEGRET complains about redundant metadata or data (now 1)
test-51-EGRET.R - convertToEGRETDaily returns partial info without preds (now 1)
test-51-EGRET.R - convertToEGRETSample correctly converts (now 1)
test-51-EGRET.R - expandFlowForEGRET returns correct columns (now 1)
IMPROVED - fewer ERRORS in these tests
--------------------------------------
(none)
DEGRADED - more ERRORS in these tests
-------------------------------------
(none)
========================================================
WARNINGS (structured, matched by file + test)
========================================================
Totals: OLD = 21 across 15 tests | NEW = 21 across 15 tests
IMPROVED - WARNINGS fully gone from these tests
-----------------------------------------------
(none)
DEGRADED - WARNINGS newly present in these tests
------------------------------------------------
(none)
IMPROVED - fewer WARNINGS in these tests
----------------------------------------
(none)
DEGRADED - more WARNINGS in these tests
---------------------------------------
(none)ALL SIX of these new errors are stopping at the same line: with the same general error message, though with differing expected and actual dateTime sizes: I currently believe that something important in EGRET changed, because the hash of EGRET that I was using had changed from the previous PR to this one: And when I install the older @99d9ffd version and rerun tests, I can get rid of the errors without reverting this PR. THEREFORE I plan to address these new EGRET-driven errors in another issue and future PR. DetailsHere are more details on the new errors:test-44-plots.R - plotEGRET works for fitdat and meta (now 1) test-51-EGRET.R - convertToEGRET allows metadata and data or newdata without model (now 1) test-51-EGRET.R - convertToEGRET complains about redundant metadata or data (now 1) test-51-EGRET.R - convertToEGRETDaily returns partial info without preds (now 1) test-51-EGRET.R - convertToEGRETSample correctly converts (now 1) test-51-EGRET.R - expandFlowForEGRET returns correct columns (now 1) |
gfkltn
left a comment
There was a problem hiding this comment.
Admin review completed. No issues found.
This PR resolves a few easy-to-fix warnings and errors, as described in the following issues:
stats::approxwhich callsstats:::regularize.values()- investigation uncovered an incorrect selection of left bounds for the horizontal lines in a rectangular interpolation, which is now fixed.count,count.days, or no count column across various models. It turns out it was intentional forpredictSolute.loadReg2to returncount.daysas already described in the doc for that method, and I fixed an inconsistency inpredictSolute.loadInterp's default for thecountargument relative to other models (though behavior is unchanged), and I fixed an error when requestingpredictSolute.loadReg2(agg.by="day")that happened to occur when trying to create thecount.dayscolumn. I left the potentially harder work in inconsistent count in predictSolute.loadReg2 output #261 for another day, as documented in that issue's final comment.These warnings, errors, and inconsistencies became visible as I was updating the vignette; I'll include a separate PR for the vignette changes. The GitHub action to build the pkgdown site should complete successfully, but it'll display an old version of the vignette.