bfast2 / bfast

Breaks For Additive Season and Trend
https://bfast2.github.io
GNU General Public License v2.0
41 stars 15 forks source link

Fix set fast options #73 #74

Closed appelmar closed 3 years ago

appelmar commented 3 years ago

This should fix R CMD check issues (see #73) with set_fast_options(). A warning is given when users call the function but do not have strucchangeRcpp installed. This check should not be necessary when we define strucchangeRcpp as an imported package.

GreatEmerald commented 3 years ago

Could we enable the options even if the regular strucchange is being linked? In that case we can give a warning that the options are not likely to have any effect, but that will make sure that we don't break the code of those who are using strucchangeRcpp as strucchange (via an earlier install_github("appelmar/strucchange", which is quite common)?

appelmar commented 3 years ago

I don't think this will affect many users because strucchangeRcpp as a new package dependency will be installed automatically when users update the bfast package (once both are on CRAN), right?

In any case, we can set the options without checking the availability. However, I've found one potential issue in the example of set_fast_options() when the "package:strucchange" environment cannot be found. It seems that this requires explicitly calling library(strucchange) before, or listing strucchange in DESCRIPTION under Depends. I'll have a closer look at this.

GreatEmerald commented 3 years ago

Yes, provided that strucchangeRcpp gets accepted... Which is still not guaranteed yet :)

And, well, we don't necessarily need to run all of the examples.

GreatEmerald commented 3 years ago

Now that strucchrangeRcpp is on CRAN, this works well, thanks!