Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backcalc_opts() rename #355

Open
seabbs opened this issue Jan 27, 2023 · 6 comments
Open

backcalc_opts() rename #355

seabbs opened this issue Jan 27, 2023 · 6 comments
Labels
high priority question Further information is requested

Comments

@seabbs
Copy link
Contributor

seabbs commented Jan 27, 2023

This is currently quite a misleading name and I think should be changed. One potential option is to call it dencolve_opts but could also call it something linking to its underlying model more closely? As this release is now looking like it will make quite a few breaking changes it would be nice to resolve this now.

It might also make sense to split the models into their own functions instead of the current approach with one perhaps being deconvolve and the other Rt? I’m not totally sure what the best names would be there. It would also make things tricky with higher level functions where they could be used interchangeably but not impossible (this is perhaps a larger change that should be saved for #352 if that goes ahead).

@seabbs seabbs changed the title backcalc_opts backcalc_opts() rename Jan 27, 2023
@seabbs seabbs added question Further information is requested high priority labels Jan 27, 2023
@sbfnk
Copy link
Contributor

sbfnk commented Jan 27, 2023

This is currently quite a misleading name and I think should be changed. One potential option is to call it dencolve_opts but could also call it something linking to its underlying model more closely? As this release is now looking like it will make quite a few breaking changes it would be nice to resolve this now.

It might also make sense to split the models into their own functions instead of the current approach with one perhaps being deconvolve and the other Rt? I’m not totally sure what the best names would be there. It would also make things tricky with higher level functions where they could be used interchangeably but not impossible (this is perhaps a larger change that should be saved for #352 if that goes ahead).

That happened #345 (see stan functions) and tentative R option interface but if going ahead that branch needs to be rebased to include the latest change and then the R option interface updated and finished).

@seabbs
Copy link
Contributor Author

seabbs commented Jan 27, 2023

Aha one (or several) steps ahead. I'd forgotten how nice this was. I'm looking at rebasing it now and getting a handle on how much work is needed.

@seabbs
Copy link
Contributor Author

seabbs commented Jan 27, 2023

Do you have thoughts about when we should try and add this in (i.e this release or next)?

Along with your suggestion for dist_spec there are a lot of breaking changes and looking over the process PR it feels like there is still a fair bit to do?

Part of me thinks we release what is on develop (which I think is breaking changes free as is) as is and then have a major release with all these breaking changes in a few weeks (basically bump up to a 2.0 release) in what can be the "paper" release.

@sbfnk
Copy link
Contributor

sbfnk commented Jan 27, 2023

Yes that sounds good to me - so 1.3.3 is what's currently in main (not on CRAN), 1.3.4 what's currently in develop (to go to CRAN), and 1.4 or 2.0 will be with an updated user interface for distributions and process but continuing with generation_interval_opts etc. so we're not intending to introduce them in one and then remove them in the next version?

@seabbs
Copy link
Contributor Author

seabbs commented Jan 30, 2023

No, I think we would have to introduce and then remove in the updated version (or perhaps remove as that is still up in the air). The only thing that would be new though is generation_time_opts() but given that it isn't compulsory to use it doesn't seem like the end of the world. We could even update the documentation to not show its usage.

@seabbs
Copy link
Contributor Author

seabbs commented Jan 30, 2023

main currently has a lot of fixes not in 1.3.3 (as released on GitHub) so is really a partial 1.3.4 (hence having an update version).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority question Further information is requested
Projects
Status: No status
Development

No branches or pull requests

2 participants