Repository navigation
Conversation
…ter keeps one level
…ls for custom hypotheses and group formulas; NEWS; bump version
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
insight::safe_deparse(comparison)can return a character vector of length > 1 (e.g., line-wrapped… · New Usingset.seed()directly in a test mutates the global RNG state and can make other tests… · New The finalexpect_identical()hard-codes label normalization behavior (e.g., parentheses removal… · New
What changed in this PR
Fixes estimate_contrasts() for ordinal-model slopes (marginaleffects backend) when a by filter leaves a single level, ensuring response-category slope contrasts are still returned instead of erroring.
Changes:
- Adjusted
format.marginaleffects_contrasts()slope-label handling to support one-levelbyfilters for response-level models. - Added regression tests for
polrandclmcovering one-level and two-levelbyfilters plus error cases. - Updated NEWS and bumped package version.
| File | Description |
|---|---|
| R/format.R | Implements one-level by handling for ordinal-slope contrasts and refactors response-level detection into a helper. |
| tests/testthat/test-ordinal.R | Adds tests validating the new behavior for ordinal slope contrasts under by filters. |
| NEWS.md | Documents the bug fix and expected behavior change. |
| DESCRIPTION | Version bump for the release containing the fix. |
Review effort: Lite
Contributor
Author
|
Copilot stopped reviewing on behalf of
jmgirard due to an error
October 19, 2026 18:47
strengejacke
approved these changes
Oct 20, 2026
Member
|
Same here, please resolve merge conflicts, I'm not sure which parts of the test file to keep |
…evel # Conflicts: # NEWS.md # tests/testthat/test-ordinal.R
…evel # Conflicts: # tests/testthat/test-ordinal.R
This branch has not been deployed
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.

If the filter in
bykept one level,estimate_contrasts()stopped with "No contrasts to show" for slopes of ordinal models with the marginaleffects backend. Ordinal models have one slope per response category, so there are still slopes to compare at that level. The call now returns the contrasts between the response categories at that level.Example, with
d <- MASS::housing,set.seed(1),d$x <- rnorm(nrow(d))andm <- MASS::polr(Sat ~ x * Infl + Type, data = d, Hess = TRUE):estimate_contrasts(m, "x", by = "Infl='Low'")estimate_contrasts(m, "x", by = "Infl='Low'", estimate = "average")estimate_contrasts(m, "x", by = "Infl=c('Low','High')")The 3 rows are Medium vs. Low, High vs. Low and High vs. Medium, with
Level1andLevel2set toLow. They equal the rows ofestimate_contrasts(m, "x", by = "Infl")where both levels areLow, with the sameDifferenceandSE. The same holds forordinal::clm().What changed, all in
R/format.R:format.marginaleffects_contrasts(). If thebyfilter leaves one level and the model has response categories, it keepsbyfor the labels and does not stop. The labels from marginaleffects then hold only the categories, such as"(Medium) - (Low)". The new.one_level_slope_labels()puts them inResponse1andResponse2, and takesLevel1andLevel2from the data grid.comparison = "b1=b2") and group formulas (~pairwise | Infl) do not get these labels, so they keep the "No contrasts to show" error..split_contrast_response_levels()moved to a helper,.is_response_level_model(), which both functions use.Tests in
test-ordinal.Rcoverpolrandclm, with"typical"and"average". They check the one-level filter, and a two-level filter against the unfiltered rows. They also check response levels with parentheses, the errors for custom hypotheses and group formulas, and the "fewer than two slopes" error forlm.Not fixed here, the same as on main:
lm(cbind(mpg, qsec) ~ wt * g)withby = "g='0'", still stop with "fewer than two slopes". That check exempts only ordinal, multinomial and categorical models.by = c("Infl", "Type")andcomparison = ~pairwise | Type, the labels of ordinal slope contrasts are wrong.