Skip to content

Conversation

@nikosbosse
Copy link
Collaborator

@nikosbosse nikosbosse commented Jan 8, 2025

Description

This PR closes #979.
This PR closes #972 .

This PR fixes the issues above by handling all renaming of columns in as_forecast_generic(). The code there became a bit more complicated, but on the other hand the individual as_forecast_() functions got simplified to the point that they become almost trivial. In a later PR we could really internalise all the logic into as_forecast_generic() and only pass in the class name as an arg.

Checklist

  • My PR is based on a package issue and I have explicitly linked it.
  • I have included the target issue or issues in the PR title as follows: issue-number: PR title
  • I have tested my changes locally.
  • I have added or updated unit tests where necessary.
  • I have updated the documentation if required.
  • I have built the package locally and run rebuilt docs using roxygen2.
  • 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.

Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice LGTM. Just needs a news entry

@nikosbosse
Copy link
Collaborator Author

Ha I hoped you would let me get away since it's just an internal thing :D I'll add a news item

@nikosbosse nikosbosse merged commit 1629541 into main Jan 8, 2025
9 checks passed
@nikosbosse nikosbosse deleted the fix-inplace-renaming branch January 8, 2025 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants