NAMESPACE issues

[jgellar]

It's not clear to me the difference between the roxygen tags @import and @importFrom. pcox currently uses @import, because when I switched to @importFrom I was getting errors that the functions I was trying to import were already in the namespace. What's the best way to do this?

Also, related, currently in order to fit a model you need to load the survival package, because a pcox formula requires a Surv() object, and the Surv object is only in survival. Should we make a pcox::Surv(...) function that just calls survival::Surv(...)?

[fabian-s]

It's not clear to me the difference between the roxygen tags @import and @importFrom. pcox currently uses @import, because when I switched to @importFrom I was getting errors that the functions I was trying to import were already in the namespace. What's the best way to do this?

could you be more specific? is it possible you were trying to import functions with the same name from different packages? i.e. do you see stuff like

Warning: replacing previous import ‘bla’ when loading ‘somePkg’

?

in general, importFrom is preferable because it only imports the specified functions and not needlessly the entire NAMESPACE. that helps to avoid naming conflicts. see the comments to the question here, for example: http://stackoverflow.com/questions/18343235/using-importfrom-in-roxygen2

Also, related, currently in order to fit a model you need to load the survival package, because a pcox formula requires a Surv() object, and the Surv object is only in survival. Should we make a pcox::Surv(...) function that just calls survival::Surv(...)?

no, that's exactly what dependencies/imports are for. survival is a base package anyway, so it's costless to just add the necessary dependency.

[jgellar]

could you be more specific? is it possible you were trying to import functions with the same name from different packages? i.e. do you see stuff like

Warning: replacing previous import ‘bla’ when loading ‘somePkg’

Yes, that's exactly what I was getting. I figured out what this means though and that the problem was I left the @import statements in the package-level documentation, and it was conflicting with the @importFrom statements in the pcox documentation. I removed these and now I am not getting those warnings.

Also, related, currently in order to fit a model you need to load the survival package, because a pcox formula requires a Surv() object, and the Surv object is only in survival. Should we make a pcox::Surv(...) function that just calls survival::Surv(...)?

no, that's exactly what dependencies/imports are for. survival is a base package anyway, so it's >costless to just add the necessary dependency.

I have @importFrom survival coxph Surv in my pcox.R file, and importFrom(survival,Surv) is in the NAMESPACE file. However, when I try to fit a model that has Surv() in the model formula (i.e., any pcox model), I get an error:

d> fit1.1 <- pcox(Surv(time, event) ~ x + male, data=dat1.1)
 Error in eval(expr, envir, enclos) : could not find function "Surv" 

The error occurs on the following line of pcox:

surv <- eval(responsename, envir = evalenv, enclos = frmlenv)

where responsename is a Call to Surv. I now understand that the problem is not a namespace issue, it was that pcox::Surv is not available to eval(). To solve it, I removed enclose=frmlenv, so now it finds Surv in the pcox environment. This same error popped up when I was executing a call to p(): it couldn't find s. The solution was the same, to remove the enclos=frmlenv.

The reason I had included enclos=frmlenv was because I thought it was necessary in case the user did not enter data, and instead wanted to refer to vectors/matrices that were sitting by themselves in their working environment. However, this doesn't seem to be the case, so it seems setting enclos is unnecessary. This makes me a little uneasy, though: could you go through the pcox code, and for all the eval statements, let me know if I should or should not be setting the enclos argument?

[jgellar]

I'm getting errors related to the coefficients and predict methods when I run check():

* checking whether the package can be loaded with stated dependencies ... WARNING
Error : object 'coefficients' not found whilst loading namespace 'pcox'
Error: package or namespace load failed for ‘pcox’
Execution halted

It looks like this package (or one of its dependent packages) has an
unstated dependence on a standard package.  All dependencies must be
declared in DESCRIPTION.
See section ‘The DESCRIPTION file’ in the ‘Writing R Extensions’
manual.
* checking whether the package can be unloaded cleanly ... WARNING
Error : object 'coefficients' not found whilst loading namespace 'pcox'
Error: package or namespace load failed for ‘pcox’
Execution halted
* checking whether the namespace can be loaded with stated dependencies ... WARNING
Error: object 'coefficients' not found whilst loading namespace 'pcox'
Execution halted

A namespace must be able to be loaded with just the base namespace
loaded: otherwise if the namespace gets loaded by a saved object, the
session will be unable to start.

Probably some imports need to be declared in the NAMESPACE file.
* checking whether the namespace can be unloaded cleanly ... OK
* checking loading without being on the library search path ... OK
* checking dependencies in R code ... NOTE
Error: package or namespace load failed for ‘pcox’
Call sequence:
2: stop(gettextf("package or namespace load failed for %s", sQuote(package)), 
       call. = FALSE, domain = NA)
1: library(package, lib.loc = lib.loc, character.only = TRUE, verbose = FALSE)
Execution halted
* checking S3 generic/method consistency ... WARNING
predict:
  function(object, ...)
predict.pcox:
  function(object, type, newdata, indices, ptimes, stimes, n, se.fit)

See section ‘Generic functions and methods’ in the ‘Writing R
Extensions’ manual.

According to ?coef, the coef generic is in the stats package, but adding stats to the Imports field in the description file did not help. I've tried to follow everything that was done for refund, and haven't found any insight in Hadley's R packages book either. What's going on?

[jgellar]

I've reduced the error to the following:

* checking whether the namespace can be loaded with stated dependencies ... WARNING
Error: object 'coefficients' not found whilst loading namespace 'pcox'
Execution halted

A namespace must be able to be loaded with just the base namespace
loaded: otherwise if the namespace gets loaded by a saved object, the
session will be unable to start.

Probably some imports need to be declared in the NAMESPACE file.

I think what reduced the error was changing survival from Imports: to Depends:, which is probably more appropriate for this package since nothing can be done without survival.

[jgellar]

Fixed the latest issue. I would still like you to take a look when you get a chance to check if I'm handling the eval() statements correctly regarding the enclos argument.

[fabian-s]

The reason I had included enclos=frmlenv was because I thought it was necessary in case the user did not enter data, and instead wanted to refer to vectors/matrices that were sitting by themselves in their working environment. However, this doesn't seem to be the case, so it seems setting enclos is unnecessary.

As stated in the docs for eval, if envir is a data.frame or list, enclos defaults to the frame from which pcox() is called (i.e., the user's "working environment"), which is what we'd want. But that should usually be identical to frmlenv <- environment(formula), unless the formula was defined somewhere else and then handed over to a function calling pcox().

When I run the first example for pcox in debug mode, both eval(responsename, envir = evalenv) and eval(responsename, envir = evalenv, enclos=frmlenv) work and yield the same result on my machine, not sure what went wrong for you there.

This makes me a little uneasy, though: could you go through the pcox code, and for all the eval statements, let me know if I should or should not be setting the enclos argument?

AFAICS it's fine as it is, no modifications required, though maybe it would make more sense to have

 assign(x = deparse(responsename),
           value = surv,
           envir = newfrmlenv)

instead of

 assign(x = deparse(responsename),
           value = eval(responsename, envir = evalenv, enclos = frmlenv),
           envir = newfrmlenv)

in lines 169 f. since we already created the Surv-object at that point. BTW: that also speaks against your diagnosis re. eval(responsename, envir = evalenv, enclos=frmlenv) --why should that NOT work in line 155 but work in line 170 .. ? :smirk: