Skip to content

Can't use offsets with offset = argument, only with offset() term - #668

Merged
strengejacke merged 8 commits into
mainfrom
strengejacke/issue667
Sep 26, 2026
Merged

Can't use offsets with offset = argument, only with offset() term#668
strengejacke merged 8 commits into
mainfrom
strengejacke/issue667

Conversation

@strengejacke

@strengejacke strengejacke commented Sep 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #667
Fixes #666

@strengejacke
strengejacke requested a lite review from Copilot September 26, 2026 10:29

This comment was marked as resolved.

Sign in to view

strengejacke and others added 4 commits September 26, 2026 12:34
@strengejacke
Fix offset handling in model call arguments
4af02d5 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@strengejacke
fixes
51a4476
@strengejacke
fixes
1e80aac
@strengejacke
test grand mean
49a6e54
@strengejacke
strengejacke requested a lite review from Copilot September 26, 2026 11:19

This comment was marked as resolved.

Sign in to view

@strengejacke
strengejacke requested a lite review from Copilot September 26, 2026 13:02
Copilot AI reviewed Sep 26, 2026
View reviewed changes

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.

Copilot review overview

🟡 Changes recommended

Unresolved offset-handling defects and a failing missing-offset test must be addressed.

Review effort: Lite
Findings: 1 Low severity

Open (1)
Resolved since last review (4)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Fix na.rmn typo so missing offsets use the non-missing mean

R/​get_marginalmeans.R:542

na.rmn is not the mean() argument na.rm, so this flag is ignored and an NA in the extracted offset leaves the datagrid offset as NA, causing prediction failures instead of using the intended mean of non-missing offsets.

This issue also appears on line 561 of the same file.

Comment thread R/get_marginalmeans.R
# if so, we use that data and don't need to do anything else. However, if
# a) model has an offset, b) newdata is provided, and c) model-offset is
# not yet included in the newdata, or explicitly specied in the offset-argument,
# we add/overwrite that value. "datagrid" is already handled above
@strengejacke
strengejacke merged commit 9b44595 into main Sep 26, 2026
12 of 21 checks passed
@strengejacke
strengejacke deleted the strengejacke/issue667 branch September 26, 2026 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Can't use offsets with offset = argument, only with offset() term Can't estimate grand mean when there's an offset

2 participants

@strengejacke