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

Add input validation #337

Closed
Tracked by #423
Bisaloo opened this issue Nov 23, 2022 · 5 comments · Fixed by #476
Closed
Tracked by #423

Add input validation #337

Bisaloo opened this issue Nov 23, 2022 · 5 comments · Fixed by #476
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Bisaloo
Copy link
Member

Bisaloo commented Nov 23, 2022

I think many errors (both in my own usage and in some bug reports such as #267) seem to come from invalid input.

I understand that what constitutes a valid input is documented but I believe some extra hand-holding by failing early with clear error messages on invalid input would go a long way in helping users.

A main topic to discuss is probably what would be the best framework to do this input checking. Some options are presented in the 'Checking the inputs of your R functions' blog post.

@seabbs seabbs added enhancement New feature or request help wanted Extra attention is needed labels Jan 17, 2023
@sbfnk
Copy link
Contributor

sbfnk commented Feb 7, 2023

One thing perhaps to do is for every of the ..._opts functions in https://github.com/epiforecasts/EpiNow2/blob/main/R/opts.R that are used to passed options to estimate_infections in

estimate_infections <- function(reported_cases,
to return their own S3 class. We could then check in estimate_infections that the arguments are of that class, avoiding instances where this is confused as e.g. in
stan = delay_opts(
.

@jamesmbaazam
Copy link
Contributor

@seabbs I would like to take a stab at this. Please assign me.

@seabbs
Copy link
Contributor

seabbs commented Feb 8, 2023

Awesome - looking forward to seeing what you come up with. Make sure to target the develop branch with your changes vs main.

@sbfnk
Copy link
Contributor

sbfnk commented May 2, 2023

There is no more develop branch so you can target main directly when taking this on.

@jamesmbaazam
Copy link
Contributor

Noted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants