Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different semantics for pars.character() in term and mbr (embr) #57

Open
krlmlr opened this issue May 29, 2023 · 2 comments
Open

Different semantics for pars.character() in term and mbr (embr) #57

krlmlr opened this issue May 29, 2023 · 2 comments
Assignees

Comments

@krlmlr
Copy link
Collaborator

krlmlr commented May 29, 2023

The Bug

This gave me pause when working on poissonconsulting/mcmcderive#2 .

A Reprex

library(term)
expr <- "
for (i in 1:nObs) {
  log(eCount[i]) <- b0 + bYear * Year[i] + bKelpLine * KelpLine[i]
}"
pars(expr)
#> Error in `term_impl()` at term/R/term.R:50:4:
#> ! All elements of term vector `string_args_term` must be valid.
#> Backtrace:
#>     ▆
#>  1. ├─universals::pars(expr)
#>  2. └─term:::pars.character(expr)
#>  3.   └─term::term(x) at term/R/pars.R:39:2
#>  4.     └─term:::term_impl(args) at term/R/term.R:50:4
#>  5.       └─term::chk_term(string_args_term, "valid") at term/R/internal.R:67:2
#>  6.         └─chk::abort_chk("All elements of term vector ", x_name, " must be valid") at term/R/chk.R:25:4
#>  7.           └─chk::err(..., n = n, tidy = tidy, class = "chk_error", call = call)
#>  8.             └─rlang::abort(msg, class = class, !!!args[named], call = call)

mbr:::pars.character(expr)
#> [1] "b0"        "bKelpLine" "bYear"     "eCount"    "i"         "KelpLine" 
#> [7] "log"       "nObs"      "Year"

Created on 2023-05-30 with reprex v2.0.2

@joethorley
Copy link
Member

Yes there is an exported terms::pars.character() that treats the input as a term vctr and an internal mbr:::pars.character() that treats the input as model code.

I was thinking that the mbr function would be hidden. We could rename it as its internal to avoid confusion?

@krlmlr
Copy link
Collaborator Author

krlmlr commented May 30, 2023

Agree to renaming it to a simple non-generic function.

@joethorley joethorley self-assigned this May 30, 2023
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

No branches or pull requests

2 participants