depreciate xpose_data?

[bguiastr]

We currently have xpose_data() for nonmem and xpose_data_nlmixr() for nlmixr. For consistency I am thinking of soft depreciating xpose_data() and rename it xpdb_nm() or xpose_nm() (or other if suggestions) we would then get similar naming for nlmixr e.g. xpdb_nlmixr() or xpose_nlmixr(). Any thoughts @andrewhooker @sebastianueckert, @MikeKSmith , @mattfidler?

[mattfidler]

I don't mind, @kestrel99 is the maintainer of xpose.nlmixr. Perhaps he can weigh in.

[MikeKSmith]

It's good to be able to say to folks migrating from Xpose4 that with xpose "just replace the dot with underscore". The ideal is that users shouldn't have to care whether the estimation has taken place with NONMEM, nlmixr or any other tool. So ultimately maybe xpose_data( ) would call xpose_data_nlmixr( ) or xpose_data_nm( ).

[mattfidler]

I think a S3 method would be appropriate to acheive this; I think that we discussed this before @guiastrennec, what do you think?

[bguiastr]

@MikeKSmith that was my initial thought but that means that the xpose_data part of the main xpose package has to account for all sub-packages and this is not great.

@mattfidler unfortunately the way I see it (correct me if I am wrong) in this case we cannot rely on S3 as we are creating the initial object and attributing its class. S3 would however work for any other function of the package as they would use an xpose as input. And this is where I want to bring xpose.

[mattfidler]

@guiastrennec

You can still use S3, the method is

##' @export
xpose_data <- function(runno = NULL,  ...){
    UseMethod("xpose_data");
}

##' @rdname xpose_data
##' @export
xpose_data.default <- function(runno = NULL, prefix = "run", ext = ".lst", file = NULL,
                               dir = NULL, gg_theme = theme_readable(), xp_theme = theme_xp_default(),
                               simtab = NULL, manual_import = NULL, ignore = NULL, extra_files, quiet,
                               ...){
    ## default import
}

This way depending on what you supply as your runno, it becomes a xpose data with nlmixr or NONMEM

You can also adjust the arguments you want to include in the default function. All methods have to include the arguments in the xpose_nlmixr to be included in CRAN.

[mattfidler]

Since nlmixr has its own class if you adopted this approach, we would not have to have any if/else for nlmixr.

[sebastianueckert]

Wouldn't it be possible for xpose.nlmixr to provide its own implementation of xpose_data? So that

library(xpose.nlmixr)
xpose_data()

invokes the nlmixr version, whereas

library(xpose)
xpose_data()

invokes the NM version. Each package still needs to provide the explicit xpose_nlmixr() and xpose_nm() to allow both packages to be loaded at the same time.

I am not in favor of the suggested S3 implementation as it appears to be a rather uncommon way of using S3. Also, it would not work if there is another software that uses a file-based data import in the future.

[bguiastr]

@mattfidler thanks for the tip this could be the way to go, and please don't get me wrong I want to change xpose to an S3 to avoid all the if else.

@sebastianueckert I think like you said there would be a conflict in xpose_data() if you were to load xpose and xpose.nlmixr together (this is actually why I created this thread to get different opinion given the importance of the function xpose_data()).

From your replies I think we have 2 options: 1) We implement the S3 method on xpose_data().

• Pros: we only have a single function that can be used with any xpose satellite package and we keep xpose as it was.
• Cons: This xpose_data() will have to be rather generic with likely only the ellipsis argument to accommodate all cases (i.e. we should be able to import different files or R objects) so the new users may struggle when searching for help with ?xpose_data() they would likely not know about ?xpose_data.nm or ?xpose_data.nlmixr. 2) We use dedicated functions for each package e.g. xpose_data_nm() and xpose_data_nlmixr().
• Pros: each method would get it own argument as needed and these would be explicitly listed in the help of the function.
• Cons: We would need to soft depreciate the previous xpose_data() and users would need to now that they are different functions to use although tab completion may ease things a bit.

In both case most xpose core functions would then be implemented as S3 to seamlessly accommodate each data class.

I may lean more toward number 2 at the moment but the debate is open :-)

[mattfidler]

Using S3 in this way is actually fairly common (common examples are print, summary, AIC , residuals, predict etc). However, @sebastianueckert does bring up a valid point that import files from another software like monolix would require something fancier. (An example would be importing the data and assigning it a specific class).

Of course xpose.nlmixr could provide its own method of xpose_data, though I'm not suggesting that at all. Without caution you may be trying to use xpose's method when you mean to use xpose.nlmixr's method and vice versa. It is cleaner to keep a single S3 class if you choose this approach.

One other point is you can keep the function exactly as it is and export it as a S3 method. In this approach you would use:

xpose_data <- function(runno = NULL, prefix = "run", ext = ".lst", file = NULL,
                       dir = NULL, gg_theme = theme_readable(), xp_theme = theme_xp_default(),
                       simtab = NULL, manual_import = NULL, ignore = NULL, extra_files, quiet,
                       ...){
    UseMethod("xpose_data");
}

xpose_data.default <- function(runno = NULL, prefix = "run", ext = ".lst", file = NULL,
                               dir = NULL, gg_theme = theme_readable(), xp_theme = theme_xp_default(),
                               simtab = NULL, manual_import = NULL, ignore = NULL, extra_files, quiet,
                               ...){
    ## default import
}

For another package to use this method, though, they would have to accept the exact same arguments, that is:

xpose_data.nlmixr <-function(runno = NULL, prefix = "run", ext = ".lst", file = NULL,
                               dir = NULL, gg_theme = theme_readable(), xp_theme = theme_xp_default(),
                               simtab = NULL, manual_import = NULL, ignore = NULL, extra_files, quiet,
                               ...){
    ## default import
}

This would take the cons away from method 1, but increase the burden of creating a S3 class in another package.

Of course, from a user's perspecitve, I think that a simple xpose_data(x) would be great if it worked everywhere.

[mattfidler]

Additionally if you want xpose.nlmixr to use xpose_data without conflict warning and without a separate s3 class, you would have to take care of Issue #95.

[MikeKSmith]

Matt - Monolix provided a method for import to Xpose4 via the DDMoRe Standard Output object. That is, they converted to SO and then we had a method of converting SO to xpdb as part of the DDMoRe R package using “as.xpdb( )”.

Sent from my iPad

On 20 Aug 2018, at 22:52, Matthew Fidler notifications@github.com wrote:

Using S3 in this way is actually fairly common (common examples are print, summary, AIC , residuals, predict etc). However, @sebastianueckert does bring up a valid point that import files from another software like monolix would require something fancier. (An example would be importing the data and assigning it a specific class).

Of course xpose.nlmixr could provide its own method of xpose_data, though I'm not suggesting that at all. Without caution you may be trying to use xpose's method when you mean to use xpose.nlmixr's method and vice versa. It is cleaner to keep a single S3 class if you choose this approach.

One other point is you can keep the function exactly as it is and export it as a S3 method. In this approach you would use:

xpose_data <- function(runno = NULL, prefix = "run", ext = ".lst", file = NULL, dir = NULL, gg_theme = theme_readable(), xp_theme = theme_xp_default(), simtab = NULL, manual_import = NULL, ignore = NULL, extra_files, quiet, ...){ UseMethod("xpose_data"); }

xpose_data.default <- function(runno = NULL, prefix = "run", ext = ".lst", file = NULL, dir = NULL, gg_theme = theme_readable(), xp_theme = theme_xp_default(), simtab = NULL, manual_import = NULL, ignore = NULL, extra_files, quiet, ...){

default import

} For another package to use this method, though, they would have to accept the exact same arguments, that is:

xpose_data.nlmixr <-function(runno = NULL, prefix = "run", ext = ".lst", file = NULL, dir = NULL, gg_theme = theme_readable(), xp_theme = theme_xp_default(), simtab = NULL, manual_import = NULL, ignore = NULL, extra_files, quiet, ...){

default import

} This would take the cons away from method 1, but increase the burden of creating a S3 class in another package.

Of course, from a user's perspecitve, I think that a simple xpose_data(x) would be great if it worked everywhere.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[mattfidler]

as.xpdb could work as well. Is it a S3 method Mike?

[mattfidler]

Perhaps as.xpdb should be a S3 method in xpose and leave everythibg else as is.

[mattfidler]

I saw you added as.xpdb, but it isn't a S3 method. This means, if I'm not mistaken, it will conflict with the ddmore package? Is this true @MikeKSmith @guiastrennec ?