Enhance lineage rendering and error handling for ggpedigree - #163
Merged
Conversation
Add robust error handling and improve interactive lineage rendering. - Wrap optimizePedigree/optimizeStaticPedigree calls in tryCatch with warnings and fallbacks so failures don't break interactive/static pipelines. - Adjust ggPedigreeInteractive to tolerate optimized failures and proceed with unoptimized plots. - Modify ggpedigreeCore logic and messages for segment_lineage in interactive mode to use fixed per-lineage colors and avoid ggnewscale conflicts; ensure lineage_active handling is consistent. - Rework ggpedigreeCoreSegments to create one fixed-colour geom_segment per lineage for plotly compatibility, support palette strings, interpolate when palette has fewer colors than needed, and render NA-lineage segments with a configured NA color. - Update tests to reflect changed behavior and add a test that verifies multiple segment colors when lineage is combined with focal_fill. - Update vignette image (binary change). These changes improve resilience against optimization failures and make segment-lineage coloring compatible with plotly interactive exports.
Adds Gael Targaryen and her stillborn son to the ASOIAF source CSV and pedigree-building script, including parent links (Gael as child of Jaehaerys I and Alysanne, stillborn as child of Gael). Regenerates `data/ASOIAF.rda` to keep packaged data in sync.
Corrects several ASOIAF pedigree records (notably Targaryen entries), including disambiguated Vaella URLs, replacing placeholder House Targaryen children with Aelor/Aelora, and adding Alys Arryn, Daenora, Maegor, and Daella’s children. Also updates coupled metadata (sex and parent mappings) and regenerates `data/ASOIAF.rda`. The vignette now uses an explicit founder ordering seed for reproducible layouts and adds a second interactive relatedness view for Rhaenyra.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves ggpedigree’s plotting robustness and interactive lineage rendering by adding config auto-repair for a common R syntax mistake (trailing commas), enhancing plotly-compatible segment-lineage coloring, and expanding/clarifying plotting configuration defaults and documentation.
Changes:
- Added
.repairTrailingCommaConfig()and wired it into multiple user-facing plotting functions to recover fromconfig = list(..., )errors. - Enhanced segment-lineage behavior in interactive/plotly mode (fallback coloring strategy) and added a new
segment_spouse_alphaconfiguration option. - Updated tests, vignettes, NEWS, exports, and dataset documentation/data to reflect new behavior and defaults.
Reviewed changes
Copilot reviewed 37 out of 57 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/articles/v22_plots_morecomplexity.Rmd | Updates extended vignette; adds founder-order config usage |
| vignettes/articles/v21_extendedinteractiveplots.Rmd | Retitles interactive vignette |
| vignettes/articles/v11_configuration_extended.Rmd | Formatting/clarity updates for founder-order examples |
| tests/testthat/test-vignette-founderOrder.R | Keeps vignette-derived tests in sync with formatting |
| tests/testthat/test-segmentLineage.R | Adds/updates interactive segment-lineage tests |
| tests/testthat/test-repairTrailingCommaConfig.R | New unit tests for trailing-comma config repair |
| tests/testthat/test-kinship2_aligns.R | Updates kinship2 alignment tests; adds ASOIAF gating/classic passthrough |
| tests/testthat/test-ggRelatednessMatrix.R | Adds trailing-comma repair test coverage |
| tests/testthat/test-ggPhenotypeByDegree.R | Adds trailing-comma repair test coverage |
| tests/testthat/test-ggPedigreeInteractive.R | Adds trailing-comma repair test coverage for interactive plots |
| tests/testthat/test-ggpedigreeCoreLayout.R | Formatting-only updates |
| tests/testthat/test-ggpedigreeCoreLabels.R | Formatting-only updates |
| tests/testthat/test-ggPedigree.R | Adds trailing-comma repair tests for static plots |
| tests/testthat/test-ggpedigree_datainputs.R | Minor formatting/comment cleanup |
| tests/testthat/test-founderOrder.R | Formatting-only updates to layout scoring tests |
| tests/testthat/test-defaultPlotConfig.R | Updates expected default-config size |
| R/repairTrailingCommaConfig.R | Implements .repairTrailingCommaConfig() helper |
| R/ggRelatednessMatrix.R | Applies trailing-comma repair to config forcing |
| R/ggPhenotypeByDegree.R | Applies trailing-comma repair to config forcing |
| R/ggPedigreeInteractive.R | Applies trailing-comma repair + adjusts optimization error handling |
| R/ggpedigreeCoreSegments.R | Adds plotly-friendly per-lineage fixed-color segment layers |
| R/ggpedigreeCoreScales.R | Forces numeric labels for focal-fill scales |
| R/ggpedigreeCore.R | Refines interactive lineage warning + adds spouse segment alpha handling |
| R/ggpedigree.R | Applies trailing-comma repair to config forcing |
| R/documentData.R | Updates ASOIAF dataset documentation metadata |
| R/defaultPlotConfig.R | Adds segment_spouse_alpha, legend logic tweaks, exports config getter |
| R/calcCoordinatesHelpers.R | Readability/formatting updates to scoring helpers |
| R/calcCoordinates.R | Readability/formatting updates + founder-order search messaging |
| NEWS.md | Reorganizes release notes; documents renames and config cleanup |
| NAMESPACE | Exports getDefaultPlotConfig |
| man/getDefaultPlotConfig.Rd | New generated documentation for exported config getter |
| man/dot-repairTrailingCommaConfig.Rd | New generated documentation for repair helper |
| man/dot-layoutScoreTwinPenalty.Rd | Spelling change in generated doc title |
| man/dot-layoutScore.Rd | Spelling change in generated docs |
| man/ASOIAF.Rd | Updates documented ASOIAF observation count |
| data-raw/wor/warsofroses.csv | Adds Wars of the Roses raw dataset CSV |
| data-raw/wor/df_raw_wor.csv | Adds Wars of the Roses raw source CSV |
| data-raw/gp/pedigree.csv | Adds large GP pedigree raw CSV |
| data-raw/df_warroses.R | Adjusts paths for war-of-roses data generation |
| data-raw/df_redsquirrels.R | Adjusts paths for redsquirrels data generation |
| data-raw/df_ASOIAF.R | Adjusts paths and updates ASOIAF data cleaning/augmentation |
| data-raw/asoiaf/ASOIAF.csv | Updates ASOIAF source CSV content |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
373
to
377
| ped_out <- best_out | ||
| if(isTRUE(config$debug)||isTRUE(config$return_best_seed)) { | ||
| if (isTRUE(config$debug) || isTRUE(config$return_best_seed)) { | ||
| message( | ||
| "Best founder order seed: ", best_seed, | ||
| " with layout score: ", best_score |
Comment on lines
+101
to
104
| if (!is.null(config$debug)) {} else { | ||
| config$debug <- debug | ||
| debug <- NULL | ||
| } |
Comment on lines
46
to
+50
| if (is.null(config$segment_lineage_palette)) { | ||
| segment_lineage_palette_colors <- paletteer::paletteer_d("khroma::bam") # default palette with good colorblind accessibility | ||
| } else if (is.character(config$segment_lineage_palette && length(config$segment_lineage_palette) == 1)) { | ||
| segment_lineage_palette_colors <- paletteer::paletteer_d(config$segment_lineage_palette) | ||
| } else if (is.character(config$segment_lineage_palette) && length(config$segment_lineage_palette) >= needed_colors) { | ||
| segment_lineage_palette_colors <- as.character( | ||
| paletteer::paletteer_d("NineteenEightyR::miami2") | ||
| ) | ||
| } else if ( |
Comment on lines
+138
to
+141
| tooltip_columns = c("ID", "name"), | ||
| founder_order_seed = 1238, | ||
| founder_order_tries = 2, | ||
| return_best_seed = TRUE |
Comment on lines
415
to
422
| test_that("classic option passes through kinship2_alignped stages with ASOIAF", { | ||
| skip_if_not_installed("quadprog") | ||
|
|
||
| # skip if not the correct data version | ||
| if (!exists("ASOIAF") || !is.data.frame(ASOIAF) || nrow(ASOIAF) != asoiaf_nrow) { | ||
| skip("ASOIAF data not available, or not the correct version") | ||
| } | ||
| data("ASOIAF") | ||
|
|
| #' @name ASOIAF | ||
| #' @usage data(ASOIAF) | ||
| #' @format A data frame with 679 observations on 9 variables. | ||
| #' @format A data frame with 688 observations on 9 variables. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Fix order-dependent ASOIAF test: move data() call before exists() check
…nt-3660917598 Fix order-dependent ASOIAF test skipping data() load before exists() check
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
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.
This pull request introduces several quality-of-life improvements, bug fixes, and new configuration options to the plotting and pedigree alignment functions. Key highlights include improved error handling for configuration issues, new segment display options, and enhanced documentation. The changes also address minor code consistency and formatting, and update the NEWS file to better organize recent updates.
Quality of Life Improvements and Bug Fixes:
ggPedigreeInteractive, ensuring the function can automatically repair and proceed when encountering malformed configs. [1] [2]ifstatements into multi-line blocks for better readability in scoring helper functions. [1] [2] [3] [4]FOO_na_valuetoFOO_na_colorfor clarity. [1] [2]Plotting Configuration Enhancements:
segment_spouse_alphato control spouse segment transparency, defaulting to semi-transparent when lineage legend is shown, and updated related documentation. [1] [2] [3]segment_lineage_includeflag.Documentation and Compatibility:
globalVariablesdeclaration forfocal_fill_na_valueto maintain compatibility with older versions. [1] [2]API and Export Updates:
getDefaultPlotConfigfunction for external use.Other Internal Improvements:
optimizePedigreefunction in both static and interactive plot generation, ensuring the pipeline continues even if optimization fails. [1] [2] [3]These changes collectively enhance the usability, robustness, and clarity of the plotting and pedigree analysis functions.