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

Keep add_names & add_ct? #41

Closed
joshwlambert opened this issue Jan 4, 2024 · 4 comments
Closed

Keep add_names & add_ct? #41

joshwlambert opened this issue Jan 4, 2024 · 4 comments
Assignees
Milestone

Comments

@joshwlambert
Copy link
Member

It was raised in the full package review for v0.1.0 (#33) that the arguments add_names and add_ct (currently used by sim_linelist() and sim_outbreak()) could be removed. Both arguments now have a default value of TRUE (since 93939ca), and thus both included in the function output.

Three options I see for development are:

  1. Retain the current implementation
  2. Remove both arguments and require postprocessing (e.g. tidyverse pipeline) to remove
  3. Move these options into config to retain flexibility for advanced users
@joshwlambert
Copy link
Member Author

I agree with the point raised in #33 that having these arguments in the function signature of sim_linelist() increases the complexity of the functions and these can be removed and we can move towards a post-processing workflow where users can call either sim_linelist() or sim_outbreak() and then use either base R or tidyverse data wrangling tools to remove or augment any line list or contacts data.

I will remove the add_ct argument from the package and add documentation advising the user on post-processing the data.

Secondly I will rename the add_names function to anonymise and change the argument default to FALSE. I believe this will be more intuitive to users.

@Bisaloo
Copy link
Member

Bisaloo commented Apr 29, 2024

I'm not entirely clear why add_names doesn't receive the same treatment as add_ct. It can be removed through the same post-process steps, can't it?

@joshwlambert
Copy link
Member Author

See the anonymise branch which builds on the rm_add_ct branch to change the add_names argument to anonymise.

@joshwlambert
Copy link
Member Author

Both PRs #104 & #106 have been merged which removed the add_ct argument and changed add_names argument to anonymise, respectively. Therefore the two points raised by this issue have been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants