Skip to content

Fix 21 logic errors in dice geometry, sizing, and category mapping#14

Open
maflot wants to merge 1 commit into
mainfrom
fix/logic-review
Open

Fix 21 logic errors in dice geometry, sizing, and category mapping#14
maflot wants to merge 1 commit into
mainfrom
fix/logic-review

Conversation

@maflot

@maflot maflot commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Bump to 1.3.0. Establish a single source of truth for category->pip-slot mapping: each data row is one pip placed at its category's factor-level index, with ndots shared between the plotted layout and the legend. This removes the paste0() id join and the reliance on scaled dots values.

Correctness fixes:

  • character dots decoded to the wrong pip position (row-order dependent)
  • n=6 plot layout did not match the legend design
  • panel/legend slot-count desync (absent levels, drop=FALSE, faceting)
  • non-injective dots palette crashed draw_panel
  • paste0(cat,x,y) id collisions drew phantom pips / cross-contaminated aes
  • pip diameters fed to GeomPoint size aesthetic (~24% too small, overflow); pips now drawn as true circleGrob mm circles
  • mapped size renormalised per panel instead of globally
  • min_fill inverted the size encoding for pip_scale < 0.25
  • NA/invalid sizes mishandled (silent drop vs full-size resurrection)

Robustness / API:

  • ndots required and validated (was an opaque base-R error)
  • forward ... to layer(); restore unknown-parameter warnings
  • validate pip_scale in (0,1]; clamp the pip-centre shift
  • per-tile tile_df aesthetics (mapping alpha/linewidth no longer crashes)
  • per-axis pip packing; proportional make_offsets padding
  • graceful fallback for non-default coordinate systems
  • clear error for >6 dot categories at build time

Docs / data:

  • geom_dice() @examples uses long-format data (comma strings were never split)
  • regenerate sample_dice_data1/2 to the documented 160-row structure

Add tests/testthat/test-logic-fixes.R covering every fixed issue. R CMD check --as-cran clean; full testthat suite passing.

Bump to 1.3.0. Establish a single source of truth for category->pip-slot
mapping: each data row is one pip placed at its category's factor-level
index, with ndots shared between the plotted layout and the legend. This
removes the paste0() id join and the reliance on scaled dots values.

Correctness fixes:
- character dots decoded to the wrong pip position (row-order dependent)
- n=6 plot layout did not match the legend design
- panel/legend slot-count desync (absent levels, drop=FALSE, faceting)
- non-injective dots palette crashed draw_panel
- paste0(cat,x,y) id collisions drew phantom pips / cross-contaminated aes
- pip diameters fed to GeomPoint size aesthetic (~24% too small, overflow);
  pips now drawn as true circleGrob mm circles
- mapped size renormalised per panel instead of globally
- min_fill inverted the size encoding for pip_scale < 0.25
- NA/invalid sizes mishandled (silent drop vs full-size resurrection)

Robustness / API:
- ndots required and validated (was an opaque base-R error)
- forward ... to layer(); restore unknown-parameter warnings
- validate pip_scale in (0,1]; clamp the pip-centre shift
- per-tile tile_df aesthetics (mapping alpha/linewidth no longer crashes)
- per-axis pip packing; proportional make_offsets padding
- graceful fallback for non-default coordinate systems
- clear error for >6 dot categories at build time

Docs / data:
- geom_dice() @examples uses long-format data (comma strings were never split)
- regenerate sample_dice_data1/2 to the documented 160-row structure

Add tests/testthat/test-logic-fixes.R covering every fixed issue.
R CMD check --as-cran clean; full testthat suite passing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant