Djm/remove fabletools

[dajmcdon]

Removes some package Imports used only in detect_outlr_stl().

• All checks pass. Some local comparisons of the guts is a few cases matched up to reasonable precision.
• Vignette output looks visually identical.
• No new tests have been added ...

[dajmcdon]

As I worked on this, is there a reason for the Collate: section of the DESCRIPTION? Does it have something to do with the sysdata.rda file in R/? Perhaps we should put that in data/ or inst/extdata to avoid.

Various other warnings unrelated to my changes in local checks.

[brookslogan]

The Collate: is to get some roxygen documentation with the same @rdname spread across multiple files to render in the way I wanted. Following up with some other questions on Slack.

[dajmcdon]

I've made the fixes based on our Slack discussion. Passes local checks, though there are some warnings due to functions I haven't touched.

I moved lubridate to Suggests. There were two imported functions that weren't used anywhere that I could find, so that is removed as well. We still use it in some tests and link to it in documentation, but it isn't needed for any code AFAICT. Maybe when CI runs, it will throw an error though.

[brookslogan]

Rebased to address merge conflicts, apply styling, and squash some commits [& force-pushed this + rebase+force-pushed again to correct some metadata]. Tested that detect_outlr example still gives effectively the same results (A simple waldo::compare(old, new) complains of differences, but at tolerance = 1e-8 will not).

[I forgot one part of the logical merge conflict resolution: moving from Abort to cli_abort. Also I still need to look over the changes.]

[brookslogan]

I think I've finished addressing merge conflicts now via rebasing. I've left a few notes here to end the day. Most are minor things that I can probably quickly clean up. But I was sidetracked by a larger surprise, which is that our method of non-seasonal fitting is sensitive to the fake seasonal settings. I can continue looking into whether there's a way to get a "cleaner" non-seasonal fit a bit, but would be glad if you have ideas/plans here. I'm quite unfamiliar with STL&LOESS.

[brookslogan]

I've once again rebased to

• add lubridate back to Imports:, as the new function epi_slide_opt() uses lubridate::as.period().

I've also pushed changes to address my feedback [+ some other things I realized but didn't note above]. I gave up on trying to actually remove the seasonal component and instead just tried to make the function slightly more flexible and make the documentation not lie.

[brookslogan]

Hi @nmdefries, could you please take a quick sanity check pass over this? I've done some git surgery on it + tried to fix a few things and it'd be helpful to have an independent pass. [Context: this was a very old refactor. Git surgery was due to the "very old" part. Non-refactor stuff was because "refactor" actually changed behavior of some (logic) branches and our documentation wasn't accurate about what was happening (neither before or after refactor); plus a little more arg validation.]

[nmdefries]

Looks reasonable enough, but it would be nice to have some tests to verify behavior. I assume existing tests would break with these changes.

[brookslogan]

Thanks for the pass. Thinking about testing:

• Golden reference / characterization tests for all of our examples and vignettes would be nice. I actually manually did this for the example + x in the outliers vignette and they actually PASS waldo::compare(tolerance = 1e-6) if you renaming "nonseasonal"/"reseasonal" to the same thing! It looks like fabletools must have inferred the same seasonality frequency we used (7).
  • testthat has support for text-based golden tests + I hacked together an RDS-based one somewhere in this repo. Maybe I should apply one of these approaches to (some fixed subsampling of?) the vignette's x.
• Other tests / actual tests in CI... I'm not sure what they should be, and I'm not sure if we might do some sort of major overhaul of outlier detection if we ever get around to focusing on the "methods" part of it. Going to forget about these for now.

[nmdefries]

IIRC, @dshemetov did some work on comparing vignettes before/after a change. I wonder if he turned up anything there that we can run automatically.

[dshemetov]

Here's what I ended up doing. I didn't keep it in, because I was concerned committing vignette outputs would balloon the repo size.

[brookslogan]

Thanks, I forgot that you already implemented something for this. Agree that if we want it in CI, it'd be better to find some way to keep the vignette outputs out of the repo (e.g., using GitHub artifacts). I ended up just visually comparing the vignettes and they look similar enough.

Again, I'm going to forget about adding other tests for now and go ahead and merge.

[brookslogan]

Approving in an [unsuccessful] attempt to wake up or regenerate CI. [Using workflow dispatch instead.]

[brookslogan]

The workflow dispatch R CMD check run run didn't attach to this PR. It has some unrelated failures that will be skipped by #480. Force-push above is to rebase on top of dev to get these test skips.