Add .col pronoun to all condformat rules (closes #19) - #44
Merged
Conversation
Every CSS-based rule (rule_fill_discrete, rule_fill_gradient, rule_fill_gradient2, rule_fill_bar, rule_text_bold, rule_text_color, rule_css) evaluated its expression once and broadcast the result identically to every selected column, so a multi-column rule call couldn't express "each column's own condition" - only a shared one, or (with no expression) a warning-and-first-column-only fallback. Add a shared eval_expression_per_column() helper (rule_helper.R) that evaluates the expression once per selected column, with a `.col` pronoun bound in the data mask to that column's own values. Wire it into all seven rules, replacing their "evaluate once, mat[, columns] <- value" pattern with a per-column loop that also recomputes any auto-detected limits/midpoint/palette from that column alone, matching the semantics of chaining the rule once per column. rule_fill_discrete(c(Sepal.Length, Sepal.Width), .col > 3) is now equivalent to chaining rule_fill_discrete(Sepal.Length, Sepal.Length > 3) and rule_fill_discrete(Sepal.Width, Sepal.Width > 3). expression now also defaults to `.col` when omitted, replacing the previous "applied to multiple columns, using column X...please use an explicit expression" warning - each column now correctly uses its own values by default instead of silently only the first one. While rewriting rule_text_bold's per-column assignment, replaced its row-selector-based matrix indexing (bold_or_not_mat[bold_or_not, columns] <- "bold") with an ifelse()-based approach that can't hit R's "NAs are not allowed in subscripted assignments" error if the expression evaluates to NA for some rows; also dropped an unused, vestigial intermediate matrix in the same function. Added regression tests for every rule confirming the .col-based single-rule call produces identical output to chaining the rule once per column, plus updated the two existing tests that asserted the now-removed multi-column warning. Updated roxygen docs, man pages (hand-synced since devtools::document() isn't available here), and the introduction vignette.
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.
Summary
Closes #19. Every CSS-based rule (
rule_fill_discrete,rule_fill_gradient,rule_fill_gradient2,rule_fill_bar,rule_text_bold,rule_text_color,rule_css) evaluated itsexpressiononce and broadcast the result identically to every selected column, so a multi-column rule call couldn't express "each column's own condition" — only a shared one, or (with no expression) a warning-and-first-column-only fallback.Added a shared
eval_expression_per_column()helper (rule_helper.R) that evaluates the expression once per selected column, with a.colpronoun bound in the data mask to that column's own values. Wired it into all seven rules, replacing each rule's "evaluate once,mat[, columns] <- value" pattern with a per-column loop that also recomputes any auto-detectedlimits/midpoint/palette from that column alone — matching the semantics of chaining the rule once per column:expressionnow also defaults to.colwhen omitted, replacing the previous "applied to multiple columns, using column X ... please use an explicit expression" warning — each column now correctly uses its own values by default instead of silently only the first one.While rewriting
rule_text_bold's per-column assignment, replaced its row-selector matrix indexing (bold_or_not_mat[bold_or_not, columns] <- "bold") with anifelse()-based approach that can't hit R's "NAs are not allowed in subscripted assignments" error if the expression evaluates toNAfor some rows; also dropped an unused, vestigial intermediate matrix in the same function.A related, separate finding (not fixed here)
While touching
rule_css.R, I noticed itsna.valueconstructor argument is accepted, documented, and stored on the rule object, butrule_to_cf_field.rule_cssnever actually readsrule[["na.value"]]— so it's a dead parameter (unlike every other rule, wherena.valueis applied). Left as-is since it's an unrelated, pre-existing gap and a behavior change of its own; happy to fix in a follow-up if wanted.Test plan
.col-based single-rule call produces output identical to chaining the rule once per columndevtools::document()isn't available in this environment — worth a sanity check withdevtools::document()before merge), and the introduction vignetteGenerated by Claude Code