Repository navigation
Sync headers from barry: fix defm formula parser covariate handling - #12
Merged
Merged
Conversation
Syncs inst/include/barry/models/defm/formula.hpp with USCbiostats/barry#26. Motif variables were read from the whole formula, so a covariate named e.g. `Day1` silently added `y1` to the motif. Also fixes `{...}x Covar` dropping the covariate, current-time rows accepted on the LHS of a transition, and unchecked rows with Markov order 1. Bumps the version to 0.2.2.9000 so dependents (defm) can require it, and adds NEWS entries for this fix and the unreleased USCbiostats/barry#25. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thank you for your contribution @gvegayon 🚀! The pkgdown site preview is ready for review 👉 Download here 👈! |
gvegayon
added a commit
to UofUEpiBio/defm
that referenced
this pull request
Sep 26, 2026
Requiring barry (>= 0.2.2.9000) broke CI: neither CRAN nor barryr's main branch provides it yet (USCbiostats/barryr#12 is still open). LinkingTo goes back to barry (>= 0.2.2). The formula parser regression tests exit early when the installed barry is older than 0.2.2.9000. They run automatically once CI installs a barry with the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gvegayon
added a commit
to UofUEpiBio/defm
that referenced
this pull request
Sep 26, 2026
* Require barry >= 0.2.2.9000 (formula parser fix) and add regression tests
barry's formula parser read motif variables from the whole td_formula()
string, including the covariate, so a covariate named e.g. `Day1` silently
added `y1` to the motif. It also dropped the covariate in `{...}x Covar`
and turned `{y0_1} > {y1}` into an intercept motif (USCbiostats/barry#26).
- Require barry (>= 0.2.2.9000), which includes the fix.
- Add inst/tinytest/test_formula_parser.R covering these cases, including
a check of the covariate-weighted statistic against a hand computation.
- Document the covariate syntax in td_formula().
- Bump to 0.2.2.9000 and add NEWS.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Depend on CRAN barry (>= 0.2.2); skip parser tests on older barry
Requiring barry (>= 0.2.2.9000) broke CI: neither CRAN nor barryr's main
branch provides it yet (USCbiostats/barryr#12 is still open). LinkingTo
goes back to barry (>= 0.2.2). The formula parser regression tests exit
early when the installed barry is older than 0.2.2.9000. They run
automatically once CI installs a barry with the fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Guard parser tests by probing the compiled parser, not barry's version
The fix is compiled into defm from barry's headers (LinkingTo), so the
installed barry version may not match what defm was built with. The tests
now try `{y0} > {y1} x Day1`, which the unfixed parser rejects, and exit
early if it fails. This works regardless of how barry is versioned.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
What
Syncs
inst/include/barry/models/defm/formula.hppwith USCbiostats/barry#26 (the only header that differs), bumps the version to 0.2.2.9000, and adds NEWS entries.Why
defm_motif_parserread motif variables from the whole formula string, including the covariate. With a covariate such asDay1,"{y0} > {y0} x Day1"silently becameMotif {a+}>{a+, b+} x Day1. The barry PR also fixes:{...}x Covar(no space beforex) silently dropping the covariate{y0_1} > {y1}silently became an intercept motif)Reviewer notes
🤖 Generated with Claude Code