Skip to content

Fix output columns that clash with focal names in contrasts, and p_adjust in estimate_slopes() (marginaleffects backend) - #683

Open
jmgirard wants to merge 7 commits into
easystats:mainfrom
jmgirard:output-column-clashes
Open

jmgirard wants to merge 7 commits into
easystats:mainfrom
jmgirard:output-column-clashes

Conversation

@jmgirard

@jmgirard jmgirard commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

This follows #679. With the marginaleffects backend, a variable named like an output column still gave wrong output. This happened in contrasts, custom hypotheses and contrasts of slopes. Also, most p_adjust options of estimate_slopes() did not work.

Name clashes (all with lm() and factors):

  • A contrast variable named Difference, t or p changed the output column with that name. The column became a factor of NA values, moved to the front, or both.
  • With a custom comparison (for example "b2=b1"), a by variable named Difference, t or p did the same.
  • The same happened in contrasts of slopes and in estimate_slopes(comparison = "b2=b1"). There, the names were Slope, SE, CI_low, CI_high, t and p.
  • In contrasts, a by variable named Slope gave two Difference columns. With comparison = "ratio", a by variable named Ratio or Slope gave two Ratio columns.

The cause was in .standardize_marginaleffects_columns() (R/format.R). data_restoretype() converted each output column to the type of the variable with the same name. The reorder moved such columns to the front. Both now apply only to focal terms that are columns of the marginaleffects output. The rename Slope to Difference in format.marginaleffects_contrasts() now runs only for contrasts of slopes, where Slope is the estimate. For ratio contrasts, the suffix check now treats Ratio as an output name. A by variable named Ratio becomes Ratio.1.

p_adjust in estimate_slopes(): eight methods failed with "replacement has 0 rows". These are "holm", "hochberg", "hommel", "bonferroni", "BH", "BY", "fdr" and "sidak". "tukey" returned unadjusted p-values ("Tukey adjustment requires at least 2 groups"). get_marginaltrends() calls .p_adjust() on the unformatted marginaleffects output. That output has p.value and statistic, but no p column. .p_adjust() now reads p.value and statistic for that output, and counts one test per row. For formatted output (contrasts), it reads the columns as before. A by variable named p now keeps its values (as p.1), and the p-values are adjusted.

Tests:

  • test-focal-name-clashes.R: a grid over 17 names and the 6 roles above. The 26 cells that stop with the rename error from Informative errors for variable names that marginaleffects overwrites (marginaleffects backend) #679 still stop with it. Each of the other 76 cells is compared with the same call for a variable named safe_var. The test compares the column names in order and every column. There are also tests for ratio ~ pairwise and for p_adjust in contrasts of estimate_relation() output.
  • test-estimate_slopes.R: each p_adjust method against stats::p.adjust(), 1 - (1 - p)^k and ptukey(), for a numeric and a factor by. Also a by variable named p, and one test per row for a single custom hypothesis and for an ordinal model.

Not fixed here: Bayesian models and inequality comparisons. Models other than lm() were not checked. by filters in contrasts of slopes fail for any variable name.

devtools::test() gives 0 failures and 0 errors locally. devtools::check() gives 0 errors and 0 warnings. The version is bumped to 0.17.0.16.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A critical ratio-contrast naming issue remains unresolved in R/format.R.

1 open finding
What changed in this PR

Fixes marginaleffects output-column name clashes and enables p_adjust methods in estimate_slopes().

Changes:

  • Preserves focal columns during formatting.
  • Adjusts unformatted marginaleffects p-values.
  • Adds regression tests, NEWS updates, and a version bump.
File Summary
tests/​testthat/​test-focal-name-clashes.R Tests focal-name clashes and contrast formatting.
tests/​testthat/​test-estimate_slopes.R Tests slope p-adjustment methods and edge cases.
R/​p_adjust.R Adjusts unformatted marginaleffects p-values.
R/​format.R Handles focal/output naming conflicts and contrast formatting.
NEWS.md Documents the fixes.
DESCRIPTION Bumps the development version.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread R/format.R

This branch has not been deployed

No deployments
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.

2 participants