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

Do not warn by default if EOL is missing in spc file #325

Closed
wants to merge 3 commits into from

Conversation

andreranza
Copy link
Collaborator

@andreranza andreranza commented Jun 10, 2024

If a spc file without EOL is given, we get a warning.

pkgload::load_all()
#> ℹ Loading seasonal

import.spc("inst/tests/trial.spc") # trial.spc has no EOL
#> Warning in readLines(file): incomplete final line found on
#> 'inst/tests/trial.spc'
#> ## import input series
#> x <- ts(c(112, 118, 132, 129, 121, 135, 148, 148, 136, 119, 104, 
#>     118, 115, 126, 141, 135, 125, 149, 170, 170, 158, 133, 114, 
#>     140, 145, 150, 178, 163, 172, 178, 199, 199, 184, 162, 146, 
#>     166), start = c(1949, 1), frequency = 12)
#> 
#> ## main call to 'seas'
#> seas(x = x, series.print = "brief", automdl = NULL, outlier = NULL, 
#>     regression.aictest = NULL)

Created on 2024-06-10 with reprex v2.1.0

With this PR, we give the choice to user whether to silence it or not. By default, we make it silent.

pkgload::load_all()
#> ℹ Loading seasonal

import.spc("inst/tests/trial.spc") # trial.spc has no EOL
#> ## import input series
#> x <- ts(c(112, 118, 132, 129, 121, 135, 148, 148, 136, 119, 104, 
#>     118, 115, 126, 141, 135, 125, 149, 170, 170, 158, 133, 114, 
#>     140, 145, 150, 178, 163, 172, 178, 199, 199, 184, 162, 146, 
#>     166), start = c(1949, 1), frequency = 12)
#> 
#> ## main call to 'seas'
#> seas(x = x, series.print = "brief", automdl = NULL, outlier = NULL, 
#>     regression.aictest = NULL)

Created on 2024-06-10 with reprex v2.1.0

Copy link
Owner

@christophsax christophsax left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @andreranza!

Do you see a downside if this is always FALSE? What is the benefit of the warning?

@andreranza
Copy link
Collaborator Author

@christophsax, not at the moment. Probably making it FALSE by default won't harm but since I wasn't sure I let the choice to the user for now. We can of course investigate a bit more.

@christophsax
Copy link
Owner

Thanks, @andreranza. I have this included in #326, without the user options. Closing this.

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.

2 participants