Skip to content

Merge lFormula/glFormula via shared mkFormula() helper#4

Open
Copilot wants to merge 7 commits into
masterfrom
copilot/merge-lformula-and-glformula
Open

Merge lFormula/glFormula via shared mkFormula() helper#4
Copilot wants to merge 7 commits into
masterfrom
copilot/merge-lformula-and-glformula

Conversation

Copilot AI commented Mar 18, 2026

Copy link
Copy Markdown

lFormula() and glFormula() shared ~90% identical implementation, noted by DRY/TODO comments in the code. This PR extracts the shared body into a private mkFormula() helper, eliminating the duplication.

Changes

  • New mkFormula() internal helper — contains all shared logic: model-frame evaluation, reTrms construction, identifiability checks, predvars extraction, and X matrix building/scaling. Behavioral differences are parameterized:

    • allow.nFALSE for lmer, TRUE for glmer
    • check_zero_rows — lmer-only "0 (non-NA) cases" guard
    • check_na_Zt — lmer-only NA-in-Z check
    • set_varnames_fixed — lmer-only varnames.fixed attribute
  • Additional cleanup from PR feedback — moved the remaining duplicated preprocessing from both public functions into mkFormula():

    • fr.form. and fr.form construction is now centralized
    • shared weights/offset environment assignment loop is now centralized
    • preserved original missing()-based argument detection behavior in caller frame
  • lFormula() — now keeps only lmer-specific setup (arg checking, family redirect, match.call() construction), delegates preprocessing/build steps to mkFormula(), appends REML

  • glFormula() — now keeps only glmer-specific setup (family handling, mustart/etastart model-frame args), delegates preprocessing/build steps to mkFormula(), sets start attribute on fr post-call, appends family

  • Removed all DRY!, TODO: merge, and ## FIXME: does start= do anything? comments that prompted this change

Public signatures of both functions are unchanged.

Original prompt

This section details on the original issue you should resolve

<issue_title>lFormula() and glFormula() clean-up</issue_title>
<issue_description>The following comments in the code:

lme4/R/modular.R

Lines 712 to 713 in 0a7f766

## TODO: remove any arguments that aren't actually used by glFormula (same for lFormula)
## TODO(?): lFormula() and glFormula() are very similar: merge or use common baseFun()
suggest that we should merge lFormula() and glFormula(). Also, should remove unnecessary arguments. </issue_description>

<agent_instructions>Try to merge lformula and glformula() without breaking anything :-)</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits March 18, 2026 20:43
… use it

Co-authored-by: annahuynhly <97189987+annahuynhly@users.noreply.github.com>
Co-authored-by: annahuynhly <97189987+annahuynhly@users.noreply.github.com>
Copilot AI changed the title [WIP] Merge lFormula() and glFormula() for cleanup Merge lFormula/glFormula via shared mkFormula() helper Mar 18, 2026
Copilot AI requested a review from annahuynhly March 18, 2026 20:46
@annahuynhly annahuynhly marked this pull request as ready for review March 20, 2026 16:45
@annahuynhly

Copy link
Copy Markdown
Owner

@copilot can you observe the issue above? I.e., the fact that fr.form and fr.form. are computed separately in glmer and lmer.

The following code is also repeated (which I did not mention earlier):

 for (i in c("weights", "offset")) {
        if (!eval(bquote(missing(x=.(i)))))
            assign(i,get(i,parent.frame()),environment(fr.form))
    }

Copilot AI and others added 3 commits March 30, 2026 13:05

Copilot AI commented Mar 30, 2026

Copy link
Copy Markdown
Author

@copilot can you observe the issue above? I.e., the fact that fr.form and fr.form. are computed separately in glmer and lmer.

The following code is also repeated (which I did...

Done in 28e5651. I moved the shared fr.form./fr.form construction and the shared weights/offset environment assignment loop into mkFormula(), so lFormula() and glFormula() no longer duplicate that block.

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.

lFormula() and glFormula() clean-up

2 participants