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

Account for the fact that element_blank() elements are length 0, but should be considered defined #83

Merged
merged 29 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
820d464
Close #82: Account for the fact that element_blank() elements are len…
cpsievert Feb 1, 2021
4fa68d6
Add a visual test
cpsievert Feb 1, 2021
3959a4f
Add link to PR
cpsievert Feb 1, 2021
f0846eb
Fix for brew cask
cpsievert Feb 1, 2021
b90a048
Appears vdiffr figs aren't being fetched
cpsievert Feb 1, 2021
2d5a800
Update new vdiffr baselines
cpsievert Feb 1, 2021
0a3c1b3
Do better at managing state in tests; fix for sf warning
cpsievert Feb 1, 2021
2c12fc3
Update news
cpsievert Feb 1, 2021
170f690
Temporarily disable shinytest
cpsievert Feb 1, 2021
8d638ab
Try pulling
cpsievert Feb 1, 2021
26218ec
Check if vdiffr tests are actually running
cpsievert Feb 1, 2021
c09c05d
Try running shinytest tests in background process; update testing dep…
cpsievert Feb 1, 2021
5fcc008
Close #79: Don't call shiny::getCurrentOutputInfo() unless shiny is a…
cpsievert Feb 1, 2021
652a693
List svg files
cpsievert Feb 1, 2021
3a20cf3
Temporarily disable R CMD check and just run tests
cpsievert Feb 1, 2021
bd3d368
Install pkg
cpsievert Feb 1, 2021
db69dbb
Propogate devtools::test() failures/errors to top-level error
cpsievert Feb 1, 2021
221803a
whoops
cpsievert Feb 1, 2021
15cf057
upload entire dir
cpsievert Feb 1, 2021
2434654
revert change to hist() baseline
cpsievert Feb 1, 2021
9a2160c
try just committing the tests
cpsievert Feb 1, 2021
b3333dd
fix test
cpsievert Feb 2, 2021
0bd0fee
update renderPlot() baselines on mac
cpsievert Feb 2, 2021
19d2db4
update win baseline
cpsievert Feb 2, 2021
03eeaeb
fix test
cpsievert Feb 2, 2021
3eb2b82
don't ignore -current folders
cpsievert Feb 2, 2021
5b7baf8
cleanup
cpsievert Feb 2, 2021
33fad1f
Add bslib to suggests
cpsievert Feb 2, 2021
0e6eaec
Also run R CMD check (but without tests)
cpsievert Feb 2, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions .github/workflows/R-CMD-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,13 @@ jobs:
- name: Mac systemdeps
if: runner.os == 'macOS'
run: |
brew cask install xquartz phantomjs
brew install --cask xquartz phantomjs
brew install pkg-config cairo

# ------------------------------------------------------------

- name: Install dependencies
run: |
install.packages("vctrs")
remotes::install_deps(dependencies = TRUE)
remotes::install_cran("rcmdcheck")
shell: Rscript {0}
Expand All @@ -117,8 +116,8 @@ jobs:
- name: Install testing dependencies
run: |
# TODO: remove me after next release cycle
remotes::install_github(c('rstudio/rmarkdown#1706', 'rstudio/bslib', 'yixuan/showtext'))
install.packages(c('shinytest', 'sf', 'ggthemes', 'patchwork', 'gridExtra', 'tinytex'))
remotes::install_github('rstudio/rmarkdown')
install.packages(c('shinytest', 'callr', 'sf', 'ggthemes', 'patchwork', 'gridExtra', 'tinytex'))
tinytex::install_tinytex()
shell: Rscript {0}

Expand Down Expand Up @@ -146,16 +145,31 @@ jobs:
Rscript
-e "cat('::set-output name=name::gha-', '${{ steps.failed_branch.outputs.name }}', '-', '${{ matrix.config.r }}', '-', '${{ runner.os }}', sep = '')"


# Run test() before R CMD check since, for some reason, rcmdcheck::rcmdcheck() skips vdiffr tests
- name: Install devtools
run: if (!require(devtools)) install.packages("devtools")
shell: Rscript {0}
- name: Run Tests
run: |
devtools::install()
res <- devtools::test()
df <- as.data.frame(res)
if (sum(df$failed) > 0 || any(df$error)) stop("GHA CI tests failed")
shell: Rscript {0}

# Run check with --no-tests since we ran them abve
- name: Check
run: rcmdcheck::rcmdcheck(args = "--no-manual", error_on = "warning", check_dir = "check")
run: rcmdcheck::rcmdcheck(args = c("--no-tests", "--no-manual"), error_on = "warning", check_dir = "check")
shell: Rscript {0}

# Upload the whole pkg since tests where run with devtools::test()
- name: Upload check results
if: failure()
uses: actions/upload-artifact@master
with:
name: ${{ runner.os }}-r${{ matrix.config.r }}-results
path: check
path: ./

- name: Push test files to a GH branch
if: failure()
Expand All @@ -172,9 +186,6 @@ jobs:

git checkout -B ${{ steps.gha_branch.outputs.name }}

cp -r check/thematic.Rcheck/tests/figs/* tests/figs/
cp -r check/thematic.Rcheck/tests/testthat/* tests/testthat/

git add tests/ && \
git commit -m 'Add test files - rstudio/thematic@${{ steps.short_sha.outputs.sha }}'

Expand Down
3 changes: 0 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ Untitled*
README.html
tmp/
tests/testthat/**/Rplots.pdf
tests/testthat/*/*/*-current
tests/testthat/*/*/*/*-current
tests/testthat/*/*/*/*/*-current
tests/testthat/testthat-problems.rds
docs/
cranwhales/
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Suggests:
rmarkdown,
htmltools,
shiny (>= 1.5.0),
bslib,
testthat,
vdiffr,
svglite,
Expand Down
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# thematic 0.1.1.9000


* Closed #82: Fixed a bug with element_blank() in plot-specific user code not being respected by thematic. (#83)

# thematic 0.1.1

Expand Down
2 changes: 1 addition & 1 deletion R/auto.R
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ auto_resolve_theme <- function(theme) {
# ------------------------------------------------------------

shiny_output_info <- function() {
if (!is_installed("shiny")) return(NULL)
if (!"shiny" %in% loadedNamespaces()) return(NULL)
info <- tryNULL(shiny::getCurrentOutputInfo())
# Return early if we're not in any output context
if (is.null(info)) return(NULL)
Expand Down
4 changes: 2 additions & 2 deletions R/ggplot.R
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,10 @@ resolve_theme_inheritance <- function(p_theme) {
parent_el <- p_theme[[this_parent]]
kid_el <- p_theme[[this_kid]]
# parent doesn't exist so do nothing
if (!length(parent_el)) {
if (is.null(parent_el)) {
next
}
p_theme[[this_kid]] <- if (length(kid_el)) {
p_theme[[this_kid]] <- if (!is.null(kid_el)) {
# both parent & child exist
ggplot2::merge_element(new = kid_el, old = parent_el)
} else {
Expand Down
3 changes: 3 additions & 0 deletions tests/figs/ggplot/no-y-text.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 1 addition & 4 deletions tests/testthat/CairoPNG/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ library(shiny)
library(ggplot2)
library(thematic)

thematic_on("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))
onStop(function() {
thematic_off()
})
thematic_shiny("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))

ui <- fluidPage(
imageOutput("CairoPNG")
Expand Down
5 changes: 1 addition & 4 deletions tests/testthat/agg_png/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ library(shiny)
library(ggplot2)
library(thematic)

thematic_on("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))
onStop(function() {
thematic_off()
})
thematic_shiny("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))

ui <- fluidPage(
imageOutput("ragg_png")
Expand Down
18 changes: 10 additions & 8 deletions tests/testthat/auto_theme_rmd/darkly_rmd/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ image_info <- function(type, ext) {
)
}

# Knit all the Rmds
# Note that this is done inside the server function
# to avoid a timeout issue with shinytest::testApp()
infile <- "darkly.Rmd"
rmd_txt <- knitr::knit_expand("../../template_theme.Rmd", theme = "darkly")
writeLines(rmd_txt, infile)
outfile <- rmarkdown::render(infile)

onStop(function() {unlink(infile)})

server <- function(input, output, session) {
# Knit all the Rmds
# Note that this is done inside the server function
# to avoid a timeout issue with shinytest::testApp()
infile <- "darkly.Rmd"
rmd_txt <- knitr::knit_expand("../../template_theme.Rmd", theme = "darkly")
writeLines(rmd_txt, infile)
outfile <- rmarkdown::render(infile)

output$ggplot <- render_image(image_info("ggplot", ext))
output$lattice <- render_image(image_info("lattice", ext))
Expand All @@ -38,7 +41,6 @@ server <- function(input, output, session) {
onFlush(function() {
unlink(dir(pattern = paste0("\\.", ext)))
unlink(c(infile, outfile, paste0(tools::file_path_sans_ext(infile), "_files")), recursive = TRUE)
unlink(infile)
})
}

Expand Down
3 changes: 1 addition & 2 deletions tests/testthat/auto_theme_shiny/base/app.R
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
library(shiny)
library(thematic)

thematic_on(font = "auto")
onStop(thematic_off)
thematic_shiny(font = "auto")

ui <- fluidPage(
titlePanel("Hello"),
Expand Down
3 changes: 1 addition & 2 deletions tests/testthat/auto_theme_shiny/lattice/app.R
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
library(shiny)
library(thematic)

thematic_on(font = "auto")
onStop(thematic_off)
thematic_shiny(font = "auto")

ui <- fluidPage(
titlePanel("Hello"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"src": "[image data sha1: 1026f88d673aaff4c59eb9c4de8d56c03550ad20]",
"width": 962,
"height": 400,
"alt": "Plot object",
"coordmap": {
"panels": [
{
Expand Down Expand Up @@ -41,6 +42,7 @@
"src": "[image data sha1: 2ca44e4a1f6cfe63be8c6f337131c54eff8814cc]",
"width": 962,
"height": 400,
"alt": "Plot object",
"coordmap": {
"panels": [
{
Expand Down Expand Up @@ -75,6 +77,7 @@
"src": "[image data sha1: 61826ec957b246af522e875b0b499045077aadf0]",
"width": 962,
"height": 400,
"alt": "Plot object",
"coordmap": {
"panels": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"src": "[image data sha1: 1026f88d673aaff4c59eb9c4de8d56c03550ad20]",
"width": 962,
"height": 400,
"alt": "Plot object",
"coordmap": {
"panels": [
{
Expand Down Expand Up @@ -41,6 +42,7 @@
"src": "[image data sha1: 2ca44e4a1f6cfe63be8c6f337131c54eff8814cc]",
"width": 962,
"height": 400,
"alt": "Plot object",
"coordmap": {
"panels": [
{
Expand Down Expand Up @@ -75,6 +77,7 @@
"src": "[image data sha1: 61826ec957b246af522e875b0b499045077aadf0]",
"width": 962,
"height": 400,
"alt": "Plot object",
"coordmap": {
"panels": [
{
Expand Down
5 changes: 1 addition & 4 deletions tests/testthat/cairo_png/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ library(shiny)
library(ggplot2)
library(thematic)

thematic_on("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))
onStop(function() {
thematic_off()
})
thematic_shiny("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))

ui <- fluidPage(
imageOutput("png")
Expand Down
5 changes: 1 addition & 4 deletions tests/testthat/cairo_svg/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ library(shiny)
library(ggplot2)
library(thematic)

thematic_on("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))
onStop(function() {
thematic_off()
})
thematic_shiny("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))

ui <- fluidPage(
imageOutput("svg")
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 2 additions & 6 deletions tests/testthat/pdf.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ output: pdf_document

```{r}
library(thematic)
thematic_on(
thematic_rmd(
"#444444", "#e4e4e4", "#749886",
font = font_spec("Liu Jian Mao Cao", scale = 1.25, update = TRUE)
)
Expand All @@ -20,9 +20,5 @@ lattice::show.settings()
```

```{r base}
image(volcano, col = thematic_get_option("sequential"))
```

```{r}
thematic_off()
image(volcano, col = thematic_get_option("sequential", resolve = FALSE))
```
5 changes: 1 addition & 4 deletions tests/testthat/quartz_png/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ library(shiny)
library(ggplot2)
library(thematic)

thematic_on("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))
onStop(function() {
thematic_off()
})
thematic_shiny("black", "white", font = font_spec("Pacifico", 1.25, update = TRUE))

ui <- fluidPage(
imageOutput("quartz")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
},
"output": {
"__reactivedoc__": {
"html": "[html data sha1: 66111e7469c463643509cdc78d840b5dbde83aeb]",
"html": "[html data sha1: bba70ec94501a4898cb719daf599f07ff70cd290]",
"deps": [
{
"name": "jquery",
Expand Down Expand Up @@ -35,7 +35,7 @@
"shim/respond.min.js"
],
"stylesheet": "css/darkly.min.css",
"head": "<style>h1 {font-size: 34px;}\n h1.title {font-size: 38px;}\n h2 {font-size: 30px;}\n h3 {font-size: 24px;}\n h4 {font-size: 18px;}\n h5 {font-size: 16px;}\n h6 {font-size: 12px;}<\/style>",
"head": "<style>h1 {font-size: 34px;}\n h1.title {font-size: 38px;}\n h2 {font-size: 30px;}\n h3 {font-size: 24px;}\n h4 {font-size: 18px;}\n h5 {font-size: 16px;}\n h6 {font-size: 12px;}\n code {color: inherit; background-color: rgba(0, 0, 0, 0.04);}\n pre:not([class]) { background-color: white }<\/style>",
"attachment": null,
"package": null,
"all_files": true
Expand Down Expand Up @@ -68,20 +68,6 @@
"package": null,
"all_files": true
},
{
"name": "anchor-sections",
"version": "1.0.1",
"src": {
"href": "anchor-sections-1.0.1"
},
"meta": null,
"script": "anchor-sections.js",
"stylesheet": "anchor-sections.css",
"head": null,
"attachment": null,
"package": null,
"all_files": true
},
{
"name": "rmarkdown-performance",
"version": "0.1",
Expand All @@ -102,6 +88,7 @@
"src": "[image data sha1: 89827a2ad6d5cbd1dc0a7742bd06cd75ad50a91c]",
"width": 910,
"height": 400,
"alt": "Plot object",
"coordmap": {
"panels": [
{
Expand Down
6 changes: 1 addition & 5 deletions tests/testthat/template_device.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,8 @@ knitr::opts_chunk$set(
fig.path = "./",
dev = {{device}}
)
```


```{r}
library(thematic)
thematic_on(
thematic_rmd(
"#444444", "#e4e4e4", "#749886",
font = font_spec("Liu Jian Mao Cao", scale = 1.25, update = TRUE)
)
Expand Down
6 changes: 1 addition & 5 deletions tests/testthat/template_theme.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ knitr::opts_chunk$set(
echo = FALSE,
fig.path = "./"
)
```

```{r}
library(thematic)
thematic_on()
thematic::thematic_rmd()
```

```{r ggplot}
Expand Down
6 changes: 4 additions & 2 deletions tests/testthat/test-base.R
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
context("base")

test_that("base baselines", {

thematic_on("black", "white", "violet", font_spec("Amatic SC", 1.5, update = TRUE))
test_that("base baselines", {
thematic_local_theme(
thematic_theme("black", "white", "violet", font_spec("Amatic SC", 1.5, update = TRUE))
)

expect_doppelganger("scatter", function() { plot(1:10) })
expect_doppelganger("scatter-cols", function() { plot(1:10, col = 1:10) })
Expand Down
Loading