ropensci / dev_guide

rOpenSci Packages: Development, Maintenance, and Peer Review
https://devguide.ropensci.org
Creative Commons Attribution Share Alike 4.0 International
158 stars 55 forks source link

Allow for cat() in combination with verbose argument #797

Closed vincentvanhees closed 8 months ago

vincentvanhees commented 8 months ago

https://github.com/ropensci/dev_guide/blob/main/pkg_building.Rmd#L62 states:

Please do not use print() or cat() unless it's for a print.*() or str.*() methods, as these methods of printing messages are harder for users to suppress.

This is not true when cat() is used in combination with a Boolean function argument named verbose, which can be an effective way of displaying progress in the console for a slow computational process. We cannot do that elegantly with message() or warning() or with a progress bar. For example: for (i in 1:10) cat(" ", i) prints the numbers next to each other horizontally in normal console text colour at each iteration of the process, while for (i in 1:10) message(" ", i) prints a vertical list taking up a lot of screen space in an alarming red colour.

I have used cat() in combination with a verbose argument when writing functions to perform computationally heavy (slow) iterative tasks where it is unclear how long the process will last. Similarly, I like to use cat() to show to the user at which stage of the time consuming process the code has arrived. It feels ineffective to use message() for this as that creates a new line for each message in alarming red colour, which I do not like as it is should be just a notification about progress.

By using cat for progress indicators the real message() and warning() calls that do deserve special attention from the user will stand out.

Proposal: Add sentence after the above sentence: "In some situations the combination of cat() and a Boolean verbose argument to skip cat() is justified, e.g. to show progress of a slow process where a simple progress bar is not adequate."

mpadge commented 8 months ago

message("...", appendLF = FALSE) does the trick. Colors can be changed with ANSI sequences, which is how the {cli} package does it. But more generally in this context, note https://github.com/ropensci/dev_guide/pull/795#issuecomment-1887130784 and comments in https://github.com/ropensci/dev_guide/pull/765 - we anticipate writing a tech note on our new recommendations for controlling output and verbosity. I'll ping you from there to ask whether you have any suggestions. Until then, i'll close this for now, with gratitude as always for the input.

vincentvanhees commented 8 months ago

Thanks. To use ANSI I would need to know what the default console text colour is of the IDE, which can be different between users (at least for RStudio). Retrieving this programmatically, if at all possible, in combination with the longer call to message() would add more code and by that make it harder to write/read/maintain. This does not make it a very appealing replacement of a cat+verbose progress indicator.

I understand the objective of the guideline but argue that this could be the one exception. Maybe suppressMessages() should instead be enhanced with an option to also suppress all cat() occurrences. I think that would be a more elegant enhancement to R than to prohibit package developers from managing verbosity at function argument level as is common in software development.

maelle commented 8 months ago

:wave: @vincentvanhees! @mpadge and I wrote a tech note on which comments are most welcome! https://ropensci.org/blog/2024/02/06/verbosity-control-packages/