Fix positioning bug in layer_symbol() and layer_text() functions, and…#163
Fix positioning bug in layer_symbol() and layer_text() functions, and…#163stemangiola merged 7 commits intomasterfrom
Conversation
… add corresponding tests. The fix ensures correct alignment of symbols/text in the heatmap matrix by using match() for row/column names. See GitHub issue #162 for details.
There was a problem hiding this comment.
Pull request overview
This pull request fixes a positioning bug in the layer_symbol() and layer_text() functions where symbols and text were incorrectly positioned in heatmap matrices. The issue occurred when factor level ordering of row/column names didn't match the actual matrix ordering, causing symbols to appear in the wrong cells (GitHub issue #162).
Changes:
- Modified
layer_symbol()to usematch()for correct position mapping based on matrix row/column names - Modified
layer_text()with the same fix for consistency - Added comprehensive unit tests to verify correct positioning behavior
- Updated version to 1.13.2 and documented the fix in NEWS.rd
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| R/functions.R | Updated layer_symbol() to use match() for correct row/column position mapping instead of factor level ordering |
| R/methods.R | Updated layer_text() with the same position mapping fix for consistency |
| tests/testthat/test-layer-positioning.R | Added new comprehensive tests to verify correct symbol positioning with issue #162 data |
| tests/testthat/_snaps/layer-positioning/layer-asterisk-issue-162.svg | Visual regression test snapshot for the positioning fix |
| inst/NEWS.rd | Documented the bug fix in version 1.13.2 release notes |
| DESCRIPTION | Bumped version from 1.13.1 to 1.13.2 |
| CRAN-SUBMISSION | Added CRAN submission tracking file (though version appears incorrect) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dummy_df <- tibble( | ||
| row_id = c( | ||
| "B cells", "Basophils", "Eosinophils", "Myeloid", "NK cells", "Neutrophils", "T cells", | ||
| "B cells", "Basophils", "Eosinophils", "Myeloid", "NK cells", "Neutrophils", "T cells" | ||
| ), | ||
| col_id = c( | ||
| rep("Control", 7), | ||
| rep("Test", 7) | ||
| ), | ||
| value = c( | ||
| 0.2000, -0.0180, 0.0983, 0.1930, 0.0934, 0.0529, -0.2090, | ||
| 0.0831, 0.00515, 0.0413, 0.0808, 0.0123, 0.0619, -0.1550 | ||
| ), | ||
| adj_p = c( | ||
| 5.81e-10, 2.72e-01, 4.46e-04, 1.44e-06, 6.43e-08, 1.08e-01, 7.65e-07, | ||
| 6.46e-02, 7.81e-01, 2.47e-01, 6.46e-02, 5.90e-01, 6.46e-02, 6.28e-03 | ||
| ) | ||
| ) %>% | ||
| mutate(col_id = fct_relevel(col_id, c("Control", "Test"))) |
There was a problem hiding this comment.
The test code is duplicated between lines 13-31 and 89-107. Both test cases create the exact same dummy_df data structure. Consider extracting this into a shared test fixture or helper function to improve maintainability and reduce code duplication.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@stemangiola I've opened a new pull request, #164, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: stemangiola <7232890+stemangiola@users.noreply.github.com>
Refactor test-layer-positioning.R to eliminate code duplication
…Ensure symbols are correctly placed for significant values and absent for non-significant ones in heatmap output.
… into fix-layers
The fix ensures correct alignment of symbols/text in the heatmap matrix by using match() for row/column names. See GitHub issue #162 for details.
Note
Fix layer positioning for symbols/text in heatmaps
layer_symbol()andlayer_text()to computerow/columnviamatch()againstabundance_matrow/column names (replacing factor-to-integer mapping) to ensure correct cell alignment (addresses layer_asterisk draws asterisks incorrectly #162).vdiffrsnapshot (layer-asterisk-issue-162.svg).NEWS(v1.13.3); addCRAN-SUBMISSIONmetadata.Written by Cursor Bugbot for commit b6c84a3. This will update automatically on new commits. Configure here.