prophet icon indicating copy to clipboard operation
prophet copied to clipboard

[R] `generated_holidays` should be internal

Open bartekch opened this issue 2 years ago • 1 comments

generated_holidays dataset is exported as data, but at the same time it is used internally. This causes issues while trying to use prophet without loading. For instance the following code would throw an error object 'generated_holidays' not found (WITHOUT loading prophet first!).

prophet::add_country_holidays(prophet::prophet() , "US")

This could be problematic even if prophet is loaded, if someone overwrites generated_holidays.

library(prophet)
generated_holidays <- 1
add_country_holidays(prophet() , "US")

I believe the solution would be to make generated_holidays internal (parameter internal = TRUE in use_data).

If dataset is still expected to be avaialble to package users, it could be tagged with @export manually (following this blogpost).

If this is acceptable approach I could make a PR with required changes.

bartekch avatar May 05 '22 15:05 bartekch

Thanks for pointing this out! Happy to go with your approach. I think it'll be good to still allow end users to access it.

tcuongd avatar May 22 '22 18:05 tcuongd