hts object overwrites underlying list of single objects after run()

[merangelik]

Example:

require(persephone)
data(pi_caladj, pi_sa, pi_unadj, weights_pi_ea19, weights_pi_eu28)
pi_caladj <- pi_caladj[ , -c(1:2)]
ts_28 <- lapply(pi_caladj, per_x13)

ts_28[[1]]
#> A persephone object
#> Not yet run.
#> 

hts_EA19 <- per_hts(list = ts_28[weights_pi_ea19$country], method = "x13")
non_EA19 <- weights_pi_eu28$country[which(!weights_pi_eu28$country %in% weights_pi_ea19$country)]
hts_EU28 <- per_hts(list = c(EA19=hts_EA19,ts_28[non_EA19]))
hts_EU28$run(verbose = FALSE)

ts_28[[1]]
#> A persephone object
#> Output:
#> class     run  run.1 class.1   seasonality log_transform arima_mdl      n_outliers q_stat
#> x13Single TRUE TRUE  x13Single Present     FALSE         (0 1 1)(0 1 1) 2          0.34  

[GregorDeCillia]

This behaviour is expected. Components are included in per_hts() by reference.

[GregorDeCillia]

After talking to @merangelik I realized that this issue was actually related to user experience. Users might find it confusing if references are used in such a manner. I suppose the bug label should be replaced by something else to clarify this.

My suggestion is therefore to refactor per_hts() such that it behaves more predictable for package users with less experience in reference semantics.

TODOs

• [ ] write a copy constructor that creates deep copies for per_hts(), per_x13() and per_tramo() objects.
• [ ] create deep copies in as.persephone()
  • NOTE: this is inconsistent with the behavior of as.data.table()
  • Alternatively, we could also call the copy method in the per_hts() constructor and leave as.persephone() as is.

[alexkowa]

Call copy in per_hts seems to be the most straight forward solution that fixes the user experience issue.

[GregorDeCillia]

It turned out that te default deep copy logic in R6 recursively copies all R6 fields. Therefore, the fix just required to create deep copies in as.persephone().

However, for deeply nested hierarchies, a lot of copies are created which could slow down the creation of per_hts objects. Therefore, we could introduce a new parameter "copy_components"in the constructor function, which allow users to switch to the old behavior.