Unexpected behaviour from drop_zero()

[athowes]

Running this minimal example I get that the drop_zero() function doesn't appear to be dropping the zeros in the data as expected:

remotes::install_github("epinowcast/epidist", dependencies = TRUE)

library(epidist)
library(data.table)

outbreak <- simulate_gillespie(seed = 101)

secondary_dist <- data.table(
  meanlog = 0.5, sdlog = 0.5
) |>
  add_natural_scale_mean_sd()

obs <- outbreak |>
  simulate_secondary(
    meanlog = secondary_dist$meanlog[[1]],
    sdlog = secondary_dist$sdlog[[1]]
  ) |>
  observe_process()

obs_drop_zero1 <- drop_zero(obs)
obs_drop_zero2 <- obs[delay_daily != 0]

> nrow(obs_drop_zero1)
[1] 8763
> nrow(obs_drop_zero2)
[1] 8456

Here is my session information. Perhaps could be to do with versions of data.table?

> sessionInfo()
R version 4.3.2 (2023-10-31)
Platform: aarch64-apple-darwin20 (64-bit)
Running under: macOS Sonoma 14.2.1

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib 
LAPACK: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libRlapack.dylib;  LAPACK version 3.11.0

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

time zone: Europe/London
tzcode source: internal

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] ggplot2_3.5.0      purrr_1.0.2        data.table_1.15.99 epidist_0.0.0.9000

[seabbs]

This looks like a bug introduced by #3 being resolved (as a filter needs a reassigment which was happening in the original piped implementation and is now not). This may have happened elsewhere so I think worth having an issue and doing a sweep!

[athowes]

Yes that looks right to me. I'm not a data.table native but I should be able to learn it and go over and fix those issues created from #3. I suppose really no need to learn just use x <- f(x); x <- g(x) rather than f(x); g(x).

[seabbs]

yes essentially some actions (filters) aren't in place and some := assignments in i are in place and so don't need assignments.

To be super safe can add lots of assignments but makes the code hard to read so probably in the first instance we want to look fro all uses of the form dt[<something>, ,] and dt[<something>] and make sure these are assigned.

Ah the joys of no unit tests...