RcppCore / RcppEigen

Rcpp integration for the Eigen templated linear algebra library
Other
111 stars 40 forks source link

Unable to create package with example_code = FALSE #75

Closed Mervap closed 4 years ago

Mervap commented 4 years ago

The following command fails RcppEigen::RcppEigen.package.skeleton(example_code = FALSE) with

Calling kitten to create basic package. Error in (function (name = "anRpackage", path = ".", author, maintainer, : unused argument (example_code = FALSE)

Error in value[3L] : error while calling kitten

It fails only if pkgKitten is installed

Mervap commented 4 years ago

Similarly with the argument force

eddelbuettel commented 4 years ago

Confirmed the example_code=FALSE case, and corrected in a branch in just pushed. Tested all four cases of example_cide equal to TRUE or FALSE with or without pkgKitten installed.

I do not know what you mean by your second remark.

Mervap commented 4 years ago

I do not know what you mean by your second remark.

I wanted to say that calling the following code

RcppEigen::RcppEigen.package.skeleton(force = TRUE)

results in a similar error

Calling kitten to create basic package. Error in (function (name = "anRpackage", path = ".", author, maintainer, : unused argument (force = TRUE)

Error in value[3L] : error while calling kitten

eddelbuettel commented 4 years ago

force is indeed an unused argument. It is there, but does nothing.

Which is what R tells you: "you gave me an argument 'force', there is nothing I can do with it". Which is correct. I should probably remove it -- or you should just not use it :)

Mervap commented 4 years ago

force is indeed an unused argument. It is there, but does nothing.

I think this argument is necessary Let's look at the documentation RcppEigen.package.skeleton

Arguments: ... force: See package.skeleton

The expected behavior is identical to package.skeleton Let's look at the documentation package.skeleton

Arguments: ... force: If ‘FALSE’ will not overwrite an existing directory.

At a minimum, the inability to create a project inside an existing folder is depressing

RcppEigen::RcppEigen.package.skeleton('testProj')

Calling kitten to create basic package. Error: Directory 'testProj' already exists. Aborting.

Error in value[3L] : error while calling kitten

Mervap commented 4 years ago

Among other things, in Rcpp::Rcpp.package.skeleton argument force is working fine and as expected

eddelbuettel commented 4 years ago

Two things:

1) Yes, force was just there because the R base package.skeleton() can use it. As you identified, it let to a second ugly error if was present AND pkgKitten was used. I just fixed that, similar to above.

2) The Error: Directory 'testProj' already exists. Aborting. is just R telling you that you directory already has a directory named testProj. It has always been like that. I was just tesing with testPkgA, testPkgB, testPkgC, ... and so on.

Mervap commented 4 years ago

I don't think we understand each other I am well aware of what these warnings mean, but I think they need to be corrected It seems logical to me to be able to create a package inside a pre-created directory And also it seems to me a terrible bug that RcppEigen::RcppEigen.package.skeleton(force = TRUE) will correctly generate the package in an existing directory if there is no pkgKitten and will crash with the error above if it is install

eddelbuettel commented 4 years ago

Please look at the branch I commited minutes ago (as I said I would; but only now pushed, sorry). It deals with the force && pkgKitten issue.

If there is anything else, please open a new issue.

I do not plan to address isues caused by R itself as both and without pkgKitten we do use R's own package.skeleton. And it will not proceed creating a new package with a given name if a directory with that name already exists.

Otherwise, please help me disentangling the different discussion threads. Thank you.

eddelbuettel commented 4 years ago

Please try now. The original issues should now be better. Thanks for bringing them up.

Mervap commented 4 years ago

Yes, it seems the original problem is solved. Thanks