[R-package] Use cat() instead of print() for metrics and callbacks

[david-cortes]

This PR modifies the callbacks that print information such as evaluation metrics to use cat() instead of print().

When using print on a character, since there is no "scalar" version of such class, R adds information about which element of the input vector is each - meaning: if there's only one element, it will always add [1] ... as part of the printed output. This is undesirable when looking at messages like evaluation metrics.

[jameslamb]

@shiyu1994 can you please go into the repository's settings at https://github.com/microsoft/LightGBM/settings/actions and confirm that "Fork pull request workflows from outside collaborators" is set to "Require approval for first-time contributors"?

@borchero and I were surprised that approval was required to run CI on the Arrow PRs, e.g. #6034 (comment) and I thought it might be specific to their account... but now I'm sure it must be something different. @david-cortes has dozens of commits in this repo, manual triggering of CI shouldn't be required on his PR.

I suspect that @borchero was right in #6034 (comment) and that someone changed this setting for this repo to "Require approval for all outside collaborators".

Sorry to bother you... if I could have admin rights in this repo, I'd also be happy to check these things myself.

[jameslamb]

Thanks as always for your interested in LightGBM, but I don't support this.

We previously were explicitly told by a CRAN reviewer to not use cat()

LightGBM/R-package/cran-comments.md

Lines 547 to 549 in fcf76bc

	> Please replace cat() by message() or warning() in your functions (except
	for print() and summary() functions). Messages and warnings can be
	suppressed if needed.

Which is why its use is blocked in this project's linting:

LightGBM/.ci/lint_r_code.R

Line 81 in fcf76bc

"cat" = "CRAN forbids the use of cat() in packages except in special cases. Use message() or warning()."

You and I have talked about this before: #4686 (comment)

I don't support tempting CRAN's ire again over an unnecessary [1] in some printed messages. I'd support other options if you'd like to explore them, but not the use of cat().

[david-cortes]

Thanks as always for your interested in LightGBM, but I don't support this.

We previously were explicitly told by a CRAN reviewer to not use cat()

LightGBM/R-package/cran-comments.md

Lines 547 to 549 in fcf76bc

	> Please replace cat() by message() or warning() in your functions (except
	for print() and summary() functions). Messages and warnings can be
	suppressed if needed.

Which is why its use is blocked in this project's linting:

LightGBM/.ci/lint_r_code.R

Line 81 in fcf76bc

"cat" = "CRAN forbids the use of cat() in packages except in special cases. Use message() or warning()."

You and I have talked about this before: #4686 (comment)

I don't support tempting CRAN's ire again over an unnecessary [1] in some printed messages. I'd support other options if you'd like to explore them, but not the use of cat().

xgboost prints them without the [1], as do dozens of other packages and ML frameworks. I am guessing that CRAN's messages were due to printing these messages in situations where it's not appropriate (like tests and doc examples). I seriously doubt that they would protest a change of print -> cat in a situation in which messages are already being printed.

[david-cortes]

As for other options not involving cat:

• Rprintf (example package doing this: cmfrec).
• lgr (example package doing this: rsparse).

[jameslamb]

I seriously doubt that they would protest a change of print -> cat in a situation in which messages are already being printed.

I've learned to not assume anything about CRAN or their willingness to be convinced by precedent.

But given that {xgboost} does do things like this...

https://github.com/dmlc/xgboost/blob/6b98305db4e09cece005ab16f9c61a3fac34258e/R-package/R/xgb.Booster.R#L776

... I guess we could at least be confident that automated checks aren't preventing the use of cat(). And in theory now that {lightgbm} is in good standing on CRAN, it shouldn't be subjected to more manual inspections.

I'm really not thrilled about taking on a bit more risk of CRAN rejecting the package in exchange for something like trimming a few characters out of a log message... but to be fair, this change would be easy to revert if it really does cause problems. We can try it.

Please remote the reference to cat in the linting configuration that I referenced, and we can move forward with this.

[david-cortes]

Removed the linter check.

[shiyu1994]

@jameslamb

@shiyu1994 can you please go into the repository's settings at https://github.com/microsoft/LightGBM/settings/actions and confirm that "Fork pull request workflows from outside collaborators" is set to "Require approval for first-time contributors"?

Done with that.

[jameslamb]

thanks @shiyu1994 !

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.