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

test commit #912

Closed
wants to merge 2 commits into from
Closed

test commit #912

wants to merge 2 commits into from

Conversation

nikosbosse
Copy link
Contributor

Description

This PR closes #.

[Describe the changes that you made in this pull request.]

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.

@nikosbosse
Copy link
Contributor Author

nikosbosse commented Sep 17, 2024

@seabbs could you do me a favour, please, and run the tests for this PR locally?

I'm getting this warning locally, but not on the CI runs:
image

The test is this:

test <- data.table::copy(example_point)
  class(test) <- c("point", "forecast", "data.table", "data.frame")
  suppressMessages(
    expect_message(
      capture.output(print(test)),
      "Could not determine forecast type due to error in validation."
    )
  )

The above is not a valid forecast object because it's first class is point instead of forecast_point. Our print.forecast() code just returns a message. But then we call NextMethod() which is print.data.table.

Within print.data.table the warning is caused by this line:

toprint = x[idx, ]

--> the subsetting triggers a revalidation, because we have a [.forecast method.

If I see correctly than this line is new (https://github.com/Rdatatable/data.table/blame/6ff4af668b064d7a2d4b597f0799eaadea2cac7f/R/print.data.table.R#L83). @MichaelChirico kindly mentioned that a data.table update was coming to CRAN (which was published on August 27).

The only thing that confuses me that this doesn't cause issues when running github actions... gh actions installs "version": "1.16.0",ofdata.table` which is the newest one...

@nikosbosse
Copy link
Contributor Author

(Note that the test above is a bit stupid anyway - it tries to artificially come up with a scenario that would actually trigger the cli message "Could not determine forecast type due to error in validation.". So if we had to remove or modify that test I think it would be completely fine.

I mostly just want to find out why there is a difference between CI and my local setup and whether that difference is relevant...)

@nikosbosse
Copy link
Contributor Author

The issue just disappeared locally. No idea what's going on.... Let's just pretend this never happened. Nothing to see here!

@nikosbosse nikosbosse closed this Sep 20, 2024
@nikosbosse nikosbosse deleted the test-pr branch September 27, 2024 17:15
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

Successfully merging this pull request may close these issues.

1 participant