Support sample weights in all circa*() functions

[ATpoint]

Dear Rex,

following up late on #7 I added sample weights support to both circacompare(), circa_single() and model_each_group() (the nls() part of it) and added a toy example to the @examples.

I intentionally did not add it to the mixed effect functions as it is unclear to me how. I think it is via something like nlme(..., weights = varPower(form=~sample_weights)) but nlme documentation and google search did not make me confident enough to add it.

Right now circacompare() and circa_single() have a new argument sample_weights that write the sample weights to the nls() input data, or when weights are NULL (the default) it simply adds a 1 per sample, so no weighting takes place.

Please have a look if you have time.

best,
Alex

[RWParsons]

Hi Alex - thanks for the PR and new feature.

I've made some feedback - could you please make those suggested changes?

Also, there are currently no new tests. Could you please add a test or two to ensure that the weights arguments are being used appropriately? For example, you could fit a model and then check that the nls model within contains a weights vector within the object.

df <- make_data(phi1 = 6)
out <- circacompare(
    x = df, col_time = "time", col_group = "group",
    col_outcome = "measure"
)
expect_true(all(out$fit$weights) == 1)

sw <- jitter(rep(1, nrow(df)), factor=2)

out2 <- circacompare(
    x = df, col_time = "time", col_group = "group",
    col_outcome = "measure", sample_weights = sw
)

expect_false(all(out2$fit$weights == 1))

or something to that effect?

Thank you!!
Rex

[RWParsons]

Also, I'm pretty sure you're right about the use of varPower in nlme. I'd be very happy if you could implement that for circa_single_mixed() and circacompare_mixed()!

The function documentation could explicitly state that these weights are passed using varPower as weights when fitting the model and refer the user to see ?nlme::nlme for further detail.

Thanks!

[ATpoint]

Hi Rex,

thanks for the review, good points made. I resolved all requested changes, implemented weights for the mixed functions and added tests for all four circa*() functions. Argument description for weights in the mixed functions now mentions that varPower() is used to pass the weights to nlme() and the tests check that this actually ends up in the fit object (fit$apVar). It's all under 0.1.1.9000 in DESCRIPTION/NEWS.

Please let me know if I missed anything or other changes are necessary.

best,
Alex

[RWParsons]

Hi Rex,

thanks for the review, good points made. I resolved all requested changes, implemented weights for the mixed functions and added tests for all four circa*() functions. Argument description for weights in the mixed functions now mentions that varPower() is used to pass the weights to nlme() and the tests check that this actually ends up in the fit object (fit$apVar). It's all under 0.1.1.9000 in DESCRIPTION/NEWS.

Please let me know if I missed anything or other changes are necessary.

best, Alex

Thanks Alex - all your changes look good to me. I've just made a couple commits but these are just formatting changes.