Issue #887 - Make `get_forecast_type()` an internal function

[nikosbosse]

Description

This PR closes #887.

This function makes get_forecast_type() an internal function. I think it's fine to have this function be internal - users can just call class(forecast) to check the type of their forecast.

to do: update manuscript and info graphics to reflect that change.

Checklist

• [x] My PR is based on a package issue and I have explicitly linked it.
• [x] I have included the target issue or issues in the PR title as follows: issue-number: PR title
• [x] I have tested my changes locally.
• [x] I have added or updated unit tests where necessary.
• [x] I have updated the documentation if required.
• [x] I have built the package locally and run rebuilt docs using roxygen2.
• [x] My code follows the established coding standards and I have run lintr::lint_package() to check for style issues introduced by my changes.
• [ ] I have added a news item linked to this PR.
• [ ] I have reviewed CI checks for this PR and addressed them as far as I am able.

[seabbs]

I don't mean to be an agent of chaos but if users can just do class(forecast) why can't we? Most of the checks could use forecast_sample etc as well as using what they currently do (i.e sample). This would mean that those implementing a new forecast class would have one less thing to do.

If we wanted we could have it as not s3 and public but just make it

get_forecast_type <- function(forecast) {
    class(forecast)[1]
}

For users who might not know about classes and might want a helper still?

If we wanted to keep the current use couldn't we just regex out the forecast_ (and error if it doesn't have that string?)?

[nikosbosse]

I don't mean to be an agent of chaos but if users can just do class(forecast) why can't we?

Yeah, fair point. I currently went for this implementation:

get_forecast_type <- function(forecast) {
  classname <- class(forecast)[1]
  if (grepl("forecast_", classname)) {
    type <- gsub("forecast_", "", classname)
    return(type)
  } else {
    cli_abort("Input is not a valid forecast object (it's first class should begin with `forecast_`).")
  }
}

I guess it's a bit of a question whether the forecast type should be "quantile" or "forecast_quantile". It feels to me that "quantile" as the forecast type has a bit more conceptual clarity. Using "forecast_quantile" might be a bit clearer code-wise.

I implemented it this way as it's just a drop-in replacement that doesn't require any additional changes to the code. Also happy to get rid of the gsub thing, but I thought maybe let's go with this for now and maybe change it later?

[seabbs]

Yes agree sounds like a good idea

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.31%. Comparing base (3d4a0ce) to head (7f8e9a3). Report is 7 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #893 +/- ## ========================================== - Coverage 98.96% 97.31% -1.66% ========================================== Files 22 22 Lines 1647 1676 +29 ========================================== + Hits 1630 1631 +1 - Misses 17 45 +28 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.