use bslib to load brand.yml files

[gordonwoodhull]

Use bslib to load brand.yml, so that brand palette colors are resolved, etc.

It would be even better to read the light and dark brands from the Quarto document's metadata, but we don't support that yet.

[cderv]

Great !

I hope there will be a brand package soon:

• feat: Add R package posit-dev/brand-yml#79

I wonder if bslib should be an import 🤔
It will really only be used with those theme_* function.

So I am thinking we could make the theme Suggest and add rlang::check_installed() in each function

rlang::check_installed("bslib", "bslib is required for brand support in R", version = "0.9.0")

This trick can be used to reduce imported packages that are installed by default and not optionally. In an interactive session, this function will offer the user the opportunity to install the missing package, which is quite nice.

Usually, I try to do this when the package is used for a specific feature only, which may concern a subset of users.

I am thinking we should have done this with the specific package targeted by each function. We assume if a user was using it they would have the suggested package installed, but probably more clean to really check for it.

[gordonwoodhull]

Good catch, bslib is suggested and not imported.

I've added rlang::check_installed for bslib and each of the theme_colors_* packages.

As we haven't released this functionality yet, I also renamed theme_*_ggplot to theme_*_ggplot2 to be correct and consistent.

[gordonwoodhull]

Thanks @cderv !

One related question I had was whether it is expected that all the Suggests will be installed whenever tests are run. I found some conflicting information like this

https://www.r-bloggers.com/2017/03/suggests-depends/

I do want automated tests that install and test these (to spot API breakage), but I think we have other tests that skip themselves if a suggested package is not installed,and I’d be glad to do that if it is best practice.

The smoke tests could easily do this.

[cderv]

I usually follow this guideline: https://r-pkgs.org/description.html#sec-description-imports-suggests

Packages listed in Imports are needed by your users at runtime and will be installed (or potentially updated) when users install your package via install.packages().

Packages listed in Suggests are either needed for development tasks or might unlock optional functionality for your users.

So anything in Suggests must conditionally used.

I do want automated tests that install and test these (to spot API breakage), but I think we have other tests that skip themselves if a suggested package is not installed, and I’d be glad to do that if it is best practice.

No problem to add them to tests. All the package 'Imports + Suggests' are installed on CI. You can then use the skip_* helper as you saw in the test so that it does not create problem in a situation where it is not installed. Which should be rare.

All these precautions are usually relevant for CRAN publishing, and its automated testing.