-
Notifications
You must be signed in to change notification settings - Fork 52
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
Fix to issue #182 #183
Fix to issue #182 #183
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Max-Bladen,
Verbosity in tests could be a good thing as this is a developer-facing matter and we need to see how every function call populates a console (or lack thereof). This also includes seeing the progress bars as this would be a good opportunity to detect any anomalies. Also, if we set (progressBar=FALSE) we are not testing that section of the code inside allif (progressBar == TRUE)
clauses. I'm happy to see some stats on how this in fact be useful to merge this.
The main reason I pushed this fix was due to the inconsistency with which these outputs were shown. It was ambiguous where they were being produced from and caused formatting errors within the check logs, making resolving unrelated issues more time consuming. I am currently working on a large overhaul/expansion of the tests (PR #206) and will include these types of parameters as part of it |
Also, the printing output can be tested without it showing in the log which in my opinion would be preferable. Rather than setting |
Looks good @Max-Bladen. Feel free to merge it once you're ready |
Within the
helpers.R
file for tests, added the.quiet()
function which prevents any calls tocat()
orprint()
within a function from being printed. Also setprogressBar = FALSE
where needed. Changed the seed withtest_tune.spls.R
such that the algorithm converged and didn't raise a warning.This allows for a much cleaner output when running tests and prevents confusion when trying to debug an issue