fix: make ui$modelName a single character string - #1170
Open
billdenney wants to merge 6 commits into
Open
Conversation
as.character() of the substituted model expression returns one element
per part of a call, so rxode2(readModelDb("PK_1cmt")) stored
c("readModelDb", "PK_1cmt") and an anonymous model function stored a
four-element vector that included the deparsed body.
.rxModelNameFromExpr() now collapses the expression to one name: a
symbol keeps its name, a call is deparsed to a single string, and an
anonymous function (with or without the `(` needed to call it) has no
name and gives NULL. The empty symbol from a missing argument stays in
that frame -- handing it to another closure raises "argument is
missing" -- and resolves to NULL.
rxUiGet.modelName normalizes on read as well, so vectors written by
released reverse dependencies (nlmixr2est's nlmixr2()/nlmixr2.rxUi()
use the same as.character(substitute()) pattern) and values in models
saved by earlier versions also come back as a single string.
Fixes nlmixr2/rxode2 issue 1019.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Returning NULL for a function definition was a special case wearing the
clothes of a rule, and it left two ways of building a model with no name
at all.
There is a uniform rule that needs no `function`-specific branch:
deparse() already breaks a multi-line construct after its opening brace,
so the first deparsed line of a function definition is "function() {".
The name is now that first line with the brace dropped and, if it is
still wider than 60 characters, truncated -- applied to every expression
alike. Symbols and calls are unchanged; an anonymous model is
"function()" rather than its body, whether it is passed to rxode2() or
called directly, and a function that arrives as a value with no
expression naming it (do.call) is "function ()".
Only a genuinely absent model argument (rxode2(filename=)) still has no
name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
billdenney
marked this pull request as ready for review
July 30, 2026 12:37
Contributor
Author
|
@mattfidler, This is ready for review. |
Member
|
This kind of breaks the assumptions from It probably should be made into a s3 method based on the function called (with a default) |
Contributor
Author
|
For my knowledge, what is the assumption within |
Member
|
No, it is also used in nlmixr2est and derives the assignment The underlying model name |
Member
|
In particular in babelmixr2 this drives the default export for NONMEM and Monolix |
Member
|
I was going to fix this in the next relase, but I need to submit to CRAN today (before the CRAN shutdown), so my fixes have to be minimal and I don't know how this affects downstream packages yet 🫤 |
`ui$modelName` came from the model expression alone, so a model from a call
was named after the text of that call (`readModelDb("PK_1cmt")`) and an
anonymous model function after a piece of its body. Both are names the
consumers of `modelName` cannot use -- nlmixr2est reports it, babelmixr2 names
NONMEM and Monolix runs with it.
`rxModelName()` is an s3 generic dispatched on the name of the function that
was called, so `rxModelName.readModelDb()` names every model `readModelDb()`
produces. The method is given the call (classed by that function name) and
the call's arguments, unevaluated and matched to the called function's
argument names; the call is shielded in `quote()` so dispatching on it cannot
re-run the function that built the model. A method that does not answer with
a single non-empty string is ignored.
`rxModelNameLhs()` registers the name an assignment is making, for an operator
like nlmixr2save's `:=`. It is used when the expression names nothing, and it
is not consumed when used, so one assignment names every model it builds.
The order is: a `rxModelName()` method, a symbol, the name being assigned to,
then the deparsed expression. An anonymous model function has no name of its
own, so it is `NULL` unless a name was registered. `rxModelNameFromExpr()`
exports the sequence for packages that capture a model expression themselves.
The merge brings 5.1.5 and 5.1.6 in as released versions, so the modelName entries written against 5.1.5 belong under 5.1.7.
rxModelName() and rxModelNameLhs() are exported and not internal, so pkgdown refused to build the reference index without them. The coverage job has been failing for months on a truncated covr trace. covr merges every covr_trace_* file left in the install path, and every R process that loads the instrumented package writes one -- including the child processes the tests start (`R CMD INSTALL` of a throw-away package, the OMP_NUM_THREADS child). A child that dies before its trace finishes writing leaves a truncated gzip stream, and the readRDS() of that one file aborts the whole merge before anything reaches Codecov. Skip the unreadable traces, naming each one that is dropped.
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
2 participants
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.
as.character() of the substituted model expression returns one element per part of a call, so rxode2(readModelDb("PK_1cmt")) stored c("readModelDb", "PK_1cmt") and an anonymous model function stored a four-element vector that included the deparsed body.
.rxModelNameFromExpr() now collapses the expression to one name: a symbol keeps its name, a call is deparsed to a single string, and an anonymous function (with or without the
(needed to call it) has no name and gives NULL. The empty symbol from a missing argument stays in that frame -- handing it to another closure raises "argument is missing" -- and resolves to NULL.rxUiGet.modelName normalizes on read as well, so vectors written by released reverse dependencies (nlmixr2est's nlmixr2()/nlmixr2.rxUi() use the same as.character(substitute()) pattern) and values in models saved by earlier versions also come back as a single string.
Fixes #1019.