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

Update documentation / default arg metrics in score() #399

Closed
nikosbosse opened this issue Nov 10, 2023 · 5 comments
Closed

Update documentation / default arg metrics in score() #399

nikosbosse opened this issue Nov 10, 2023 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@nikosbosse
Copy link
Contributor

Suggestion would be to have metrics be an argument of score(), i.e. score <- function(data, metrics, ...) which then has a default for all the methods.

For the documentation, could write something like

#' @param metrics An optional named list of scoring functions. Names will be used as
#' column names in the output. See [metrics_point()], [metrics_binary()],
#' `metrics_quantile()`, and [metrics_sample()] for more information on the
#' default metrics used.
@nikosbosse nikosbosse converted this from a draft issue Nov 10, 2023
@nikosbosse nikosbosse added this to the scoringutils 2.0.0 milestone Nov 10, 2023
@nikosbosse
Copy link
Contributor Author

I tested this and it didn't really work. The problem is in score.default():

Either metrics doesn't get passed down to the actual method, as in this case:

score.default <- function(data, metrics, ...) {
  data <- validate(data)
  score(data, ...)
}

or metrics can't be empty and it will error if it's missing, as in this case:

score.default <- function(data, metrics, ...) {
  data <- validate(data)
  score(data, metrics, ...)
}

We can still have the description be "an optional list", but I'm not sure adding metrics as an argument to score() works very well / we'd have to make it work in a way that I'm not sure adds that much

@nikosbosse nikosbosse added documentation Improvements or additions to documentation and removed next-release labels Nov 18, 2023
@nikosbosse
Copy link
Contributor Author

@seabbs Do you care strongly about this or do you think we can close this for? I don't really have a strong preference, but if you care I'm happy to keep it open

@nikosbosse
Copy link
Contributor Author

Closing this for now, but please feel free to re-open

@seabbs
Copy link
Contributor

seabbs commented Nov 27, 2023

So this surfaces something we could think about which would be forcing the user to explicitly call as_forecast themselves vs doing it for them in score.default this would get us away from having to call score inside score which is a little abusive.

@nikosbosse
Copy link
Contributor Author

I opened up a new issue, #507 which I think mostly includes this issue as well:

  • if we decide to force users to call as_forecast(), then metrics should be an argument of score()
  • if we don't then we leave it out as an argument

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Archived in project
Development

No branches or pull requests

2 participants