-
Notifications
You must be signed in to change notification settings - Fork 0
Covariate layer #19
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
base: main
Are you sure you want to change the base?
Covariate layer #19
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a covariate layer in the header formatting, adds tests and example scripts, updates documentation, and extends utility functions for custom formatting.
- Adds a
format_covariate_header
function and integrates it into thetbl_format_header.SE_print_abstraction
method. - Introduces new unit tests under
tests/testthat/
and a standalone debug script. - Updates the README examples and session info, and extends pillar utilities and NAMESPACE imports.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
vignettes/Introduction.R | Deleted outdated introduction vignette |
tests/testthat/test-header-formatting.R | Added snapshot tests for format_covariate_header |
test_header_formatting.R | Added standalone debug/example script (should be reviewed) |
README.md | Updated example output rows and sessionInfo details |
R/tidyprint_1_utlis.R | Added format_covariate_header and extended formatting logic |
R/print_methods.R | Adjusted separator row computation and attribute packing |
R/pillar_utlis.R | Extended pillar___format_comment with strip.spaces |
NAMESPACE | Added new importFrom entries for dplyr, purrr, etc. |
Comments suppressed due to low confidence (3)
R/pillar_utlis.R:3
- This function now calls
get_extent
, butget_extent
isn't imported in NAMESPACE; addimportFrom(pillar, get_extent)
so the function resolves correctly at runtime.
pillar___format_comment <- function (x, width, strip.spaces = TRUE)
NAMESPACE:19
- Missing imports for newly used functions
pillar::style_subtle
andpillar::get_extent
; please addimportFrom(pillar, style_subtle, get_extent)
to NAMESPACE.
importFrom(cli,col_br_black)
test_header_formatting.R
Outdated
@@ -0,0 +1,139 @@ | |||
# Test script for header formatting function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This standalone debug/example script appears to be for local testing; consider removing it or moving it under tests/
so it does not clutter the root directory.
Copilot uses AI. Check for mistakes.
R/tidyprint_1_utlis.R
Outdated
|
||
covariate_indices <- which(printed_colnames %in% covariate_names) | ||
covariate_widths <- separator_row[printed_colnames[covariate_indices]] |> purrr::map_int(nchar) | ||
total_covariate_width <- sum(covariate_widths) + length(covariate_widths) + 3 # To compensate the white spaces of the tibble |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using unexplained magic numbers like + 3
; consider computing the extra spacing based on actual separator characters or document the reason for this offset.
total_covariate_width <- sum(covariate_widths) + length(covariate_widths) + 3 # To compensate the white spaces of the tibble | |
# Compute extra spacing dynamically based on separator characters | |
separator_spacing <- nchar("|") * 2 + 1 # Account for '|' at both ends and one space | |
total_covariate_width <- sum(covariate_widths) + length(covariate_widths) + separator_spacing |
Copilot uses AI. Check for mistakes.
R/tidyprint_1_utlis.R
Outdated
header_row[covariate_indices[-1]] <- "" | ||
|
||
# Add row ID spacing at the beginning | ||
header_row <- c(paste(rep(" ", number_of_total_rows |> nchar() - 3), collapse = ""), header_row) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calculating row-ID spacing using nchar(number_of_total_rows) - 3
is brittle; consider deriving the padding from the actual row ID width rather than hard-coding - 3
.
header_row <- c(paste(rep(" ", number_of_total_rows |> nchar() - 3), collapse = ""), header_row) | |
row_id_width <- nchar(total_rows) | |
header_row <- c(paste(rep(" ", row_id_width), collapse = ""), header_row) |
Copilot uses AI. Check for mistakes.
number_of_features, | ||
number_of_samples, | ||
assay_names %>% paste(collapse=", ") | ||
)), after = 1) | ||
))) | ||
# , after = 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove leftover commented-out code snippets to keep the implementation clean and maintainable.
# , after = 1) |
Copilot uses AI. Check for mistakes.
…riate_header function; update NAMESPACE accordingly.
bb88d85
to
b69627c
Compare
…or format_covariate_header
…int method for SummarizedExperiment to improve design handling and output formatting.
…label alignment and improve overall output presentation.
No description provided.