Fix/linewidth#170
Open
MMenninger wants to merge 7 commits into
Open
Conversation
From AI -- Problem. The whole working tree currently shows as modified in git, but git diff --ignore-all-space shows zero real changes — it is pure CRLF↔LF line-ending noise. Without fixing this first, every later PR diff is buried under thousands of phantom line-ending changes and nothing reviews cleanly. Change. Add a .gitattributes that forces LF for text files (* text=auto eol=lf plus explicit entries for .R, .Rmd, .Rd, .md, .yaml/.yml, DESCRIPTION, NAMESPACE, .Rproj) and marks binaries (.rda, .png, .ico, .svg) as binary. Set the RStudio project to write LF (LineEndingConversion: Posix in cmapplot.Rproj) so RStudio doesn't re-introduce CRLF on save. Run git add --renormalize . in the same commit. Files. .gitattributes (new), cmapplot.Rproj. Review note. The diff looks huge but is mechanical — "line endings only."
Problem. R/cmapplot.R (~lines 110–131) gates graphics setup on
rstudioapi::isAvailable() / rstudioapi::getVersion(). In Positron,
isAvailable() returns TRUE (Positron ships a partial rstudioapi shim), so
execution enters the RStudio branch, where getVersion() can error outside real
RStudio — risking an error on package load. The RStudioGD.backend option is
meaningless to Positron (its Ark device already auto-uses ragg), and the
"please install RStudio" fallback message is wrong advice for Positron users.
Change. Detect the IDE via environment variables instead:
RStudio: Sys.getenv("RSTUDIO") == "1"
Positron: Sys.getenv("POSITRON_VERSION") != ""
In RStudio, keep the existing RStudioGD.backend = "ragg" behavior. In
Positron, skip the option entirely and drop the RStudio-install message. Keep
the plain-R fallback message.
Files. R/cmapplot.R
The culprit is the caption in finalize.Rmd:
rcaption = "Source: Chicago Metropolitan Agency for Planning
analysis of Regional Transportation Authority data"
That second line is indented ~16 spaces to line up in the source. gridtext::textbox_grob() parses captions/titles as markdown, and in markdown a line break followed by 4+ spaces means "indented code block" → it emits a <pre> tag. A newer gridtext now hard-errors on <pre> (textbox_grob only supports a few tags), so the vignette build dies. Older gridtext tolerated it; the CI runner just installed a stricter version. Pre-existing landmine, newly triggered.
Problem. Rcpp is in Imports: and @imported, but nothing calls it. There is no src/, no .cpp, no LinkingTo. The compiled routines used by geom_text_lastonly_repel() are ggrepel's, called via base .Call() — which does not need Rcpp. The dependency is vestigial. Change. Remove Rcpp, from Imports: in DESCRIPTION. Remove Rcpp from the @import tag in R/cmapplot.R (~line 14). Re-document to drop import(Rcpp) from NAMESPACE. Files. DESCRIPTION, R/cmapplot.R, NAMESPACE.
Problem. The manual Whitney-font fallback in R/cmapplot.R (~line 48) only checks ~/Library/Fonts (macOS). Windows and Linux users get no fallback when systemfonts::system_fonts() misses the fonts. Change. Extend the fallback to also check the user font directories on Windows (%LOCALAPPDATA%\Microsoft\Windows\Fonts, C:/Windows/Fonts) and Linux (~/.fonts, ~/.local/share/fonts). Files. R/cmapplot.R.
PR 5 — Modernize GitHub Actions + ggplot2 release/devel matrix (START HERE) .github/workflows/check-standard.yaml and pkgdown.yaml use actions/checkout@v3, actions/cache@v3, and actions/upload-artifact@main (a floating tag) and skip Linux. Bump to current pinned action versions; replace @main. Add an Ubuntu runner. Add a ggplot2 release + devel matrix (the r-lib/actions/check-r-package template supports this) so ggplot2 breakage is caught before it reaches a user mid-render. check-standard.yaml — full rewrite onto the modern r-lib/actions v2 pattern: Replaced the hand-rolled remotes + manual cache + rcmdcheck steps with setup-r-dependencies + check-r-package (handles caching, system deps, and result upload internally). Killed the risky actions/upload-artifact@main floating tag entirely (the modern action manages artifacts itself). Added an OS matrix: macOS, Windows, and Ubuntu (previously commented out). Added backward + forward compat: oldrel-1 (guards your R >= 3.5 floor) and a ggplot2-devel canary — an Ubuntu job that installs github::tidyverse/ggplot2 and is marked continue-on-error: true, so it reports upcoming ggplot2 breakage (the theme_set() and #164 class of problems) without blocking merges. Set error-on: '"error"' to match the repo's prior behavior — so existing size-deprecation warnings don't turn everything red before PR 6 lands. pkgdown.yaml — minimal safe bump: actions/checkout@v3 → v4 (both spots) and actions/cache@v3 → v4. The private CMAP-fonts checkout, the font-install step, and both deploy_to_branch steps are untouched. One judgment call to flag The ggplot2-devel job is deliberately non-blocking. That's the right default for a canary, but it means its signal only shows if someone looks at the checks list — it won't fail a PR. If you'd rather it be load-bearing once #164 and the theme work are fixed, flip allow_failure off in a later PR. I noted the reasoning in a comment in the file.
Changed to linewidth: theme_cmap.R — geom_hline/geom_vline (the origin lines), all six element_line() calls (gridlines, axis lines, ticks), the base line element, and the element_rect() border in theme_cmap_base(). default_aes.R — the GeomLine default only. geom_recessions.R / geom_pandemics.R — the GeomRecessions/GeomPandemics default_aes (size = 0.5 → linewidth = 0.5) and their draw code (lwd = coords$size → coords$linewidth), kept internally consistent. Left as size (correctly): all element_text() font sizes, the text-geom defaults (3.88, fsize/.pt), and the point-geom sizes (GeomPointLast, and the coords$size in the point-drawing code). DESCRIPTION: bumped ggplot2 to >= 3.4.0 in Depends — linewidth was introduced there, so this is now the real floor. Each replacement ran with an exact occurrence-count assertion, so nothing over-matched, and all four R files are brace-balanced and untruncated. Fix typo: theme_cmap_base() read $fgiont instead of $font The `text` element in theme_cmap_base() referenced cmapplot_globals$fgiont$regular$face, which resolved to NULL and dropped the base theme's default font face. Corrected to $font.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.