correct package building makefile

[chrisdane]

Hi

I have two questions. 1) After adding another GSW function (gsw_distance) to the repo, I hit

$ Rscript -e "roxygen2::roxygenise()"
Loading gsw
Loading required package: testthat
Warning: The existing 'NAMESPACE' file was not generated by roxygen2, and will not be overwritten.

Is this ok? git status does not mention the NAMESPACE file.

2) Then, I run this Makefile

## my makefile
mygsw: export R_LIBS_USER := ~/my/package/path/ # this is necessary to find knitr package
mygsw: 
    $(eval GSWVSN := $(shell awk '/Version/{print($$2)}' GSW-R/DESCRIPTION))
    @echo gsw version: $(GSWVSN) 
    @echo R_LIBS_USER: $(R_LIBS_USER) 
    cd GSW-R 
    echo "devtools::document(roclets=c('rd', 'collate', 'vignette'))" | R --no-save PKG_CFLAGS=--pedantic R CMD build --compact-vignettes="gs+qpdf" GSW-R R CMD CHECK --as-cran gsw_${GSWVSN}.tar.gz R CMD INSTALL gsw_${GSWVSN}.tar.gz

which gives

$ make mygsw 
gsw version: 1.0-6
R_LIBS_USER: ~/my/package/path/
cd GSW-R 
echo "devtools::document(roclets=c('rd', 'collate', 'vignette'))" | R --no-save PKG_CFLAGS=--pedantic R CMD build --compact-vignettes="gs+qpdf" GSW-R R CMD CHECK --as-cran gsw_1.0-6.tar.gz R CMD INSTALL gsw_1.0-6.tar.gz
Warning: unknown option ‘--as-cran’
* checking for file ‘GSW-R/DESCRIPTION’ ... OK
* preparing ‘gsw’:
* checking DESCRIPTION meta-information ... OK
* cleaning src
* installing the package to build vignettes
* creating vignettes ... OK
* cleaning src
* checking for LF line-endings in source and make files and shell scripts
* checking for empty or unneeded directories
* looking to see if a ‘data/datalist’ file should be added
* building ‘gsw_1.0-6.tar.gz’

 ERROR
cannot change to directory ‘R’
make: *** [Makefile:8: mygsw] Error 1

I definitely incorporated some errors in my Makefile (note also the unknown --as-cran). However, the provided example at the end of develop/developer/README.md did not work for me.

Kind regards, Chris

[dankelley]

Your makefile is mixed up, with the stringing of R commands like that.

The normal way to build R packages is e.g.

cd GSW-R ; echo "devtools::document(roclets=c('rd', 'collate', 'vignette'))" | R --no-save
PKG_CFLAGS=--pedantic R CMD BUILD --compact-vignettes="gs+qpdf" GSW-R
R CMD CHECK --as-cran gsw_1.0-6.tar.gz
R CMD INSTALL gsw_1.0-6.tar.gz

at the unix commandline. I am not sure whether you can do as-cran because you are not assigned as the author in the DESCRIPTION file, so maybe drop that phrase in the third line.

Obviously, your version number may differ.

PS. most people build packages in RStudio, and that's a good way to start, since your mention of the NAMESPACE file seems to indicate that you are not yet familiar with R.

[richardsc]

Hi @chrisdane. Thanks for working to add more functions to GSW-R!

Building packages can be a bit tricky if you're new to it, but as @dankelley there are easy ways to do it with existing tools (e.g. RStudio and RStudio "projects") that can help a lot.

First, how are you doing your additions to the code? The best way is to "fork" the repository on GitHub, which creates a copy of it in your own Github account. Then you can work/commit/push all you want, as if it was your own repo. However, the big advantage to this is then when it comes time to propose the addition back into this (the main, or "upstream") repo, you can create a "Pull Request" through GH that will make the task of merging your changes as seamless as possible, while also providing the opportunity for discussion (by creating an "issue" thread much like this one).

Another advantage to a fork is it gives other developers a chance to look at (or test, comment on, etc) your changes before actually trying to merge.

As for your questions:

1. The NAMESPACE warning occurs because we use the roxygen2 package to do the function documentation right beside the code, however we opted (for some reason I don't remember) to manually create and update the NAMESPACE file. What that probably means for you is that to have your "new" function available when the package is loaded you'll have to add it manually to the NAMESPACE.

2. As @dankelley suggested, I think your best bet is to use RStudio to do the package building. For some packages I have a simple Makefile that I use if for some reason RStudio isn't doing what I want (happens sometimes, not sure why ...). It looks like this:

   
   OCEVSN=$(shell grep Version oce/DESCRIPTION | awk '{print $$2}')

oce: -rm oce*.tar.gz R CMD build oce R CMD install oce$(OCEVSN).tar.gz

oce-cran: -rm oce_*.tar.gz PKGCFLAGS=--pedantic R CMD BUILD oce R CMD CHECK --as-cran oce${OCEVSN}.tar.gz R CMD INSTALL oce_${OCEVSN}.tar.gz

clean: -rm *.tar.gz

Hope that helps!

[chrisdane]

Hi

Thanks for your helpful answers.

Regarding the NAMESPACE file, apparantly it was not created by roxygen2, as the header

# Generated by roxygen2: do not edit by hand

is missing in the NAMESPACE file. Maybe this is the reason why I cannot update the NAMESPACE file?:

$ Rscript -e "roxygen2::roxygenise()"
Loading gsw
Loading required package: testthat
Warning: The existing 'NAMESPACE' file was not generated by roxygen2, and will not be overwritten.

Instead, if I delete NAMESPACE to create a new one, the following error appears:

$ rm NAMESPACE
$ Rscript -e "roxygen2::roxygenise()"
Writing NAMESPACE
Loading gsw
Loading required package: testthat
Error in .C("set_up_gsw_data", as.integer(saar$gsw_nx), as.integer(saar$gsw_ny),  : 
  "set_up_gsw_data" not resolved from current namespace (gsw)
Calls: <Anonymous> ... load_code -> <Anonymous> -> run_pkg_hook -> <Anonymous> -> .C
Execution halted

which appears to be related to R/zzz.R, which, however, I did not touch at all.

Note that R -e 'library(devtools);document()' shows the same behavior.

I forked the repo now. It would be great if you could give me a git command hint how to transfer all my changes made in my local develop branch of this repo into my forked repo.

Thank you very much for your newbie-guidance! Chris

[richardsc]

Hi Chris,

This is a bit tricky. In fact, you should update the NAMESPACE by hand because roxygen2 knows that it didn't create it and so won't try and make a new one. We could update the roxygen documentation to do the NAMESPACE generation, but that will require updates to every function and we haven't gotten to it.

I don't know of any good git hints for moving changes from one repo to another (and preserving the history). The easiest thing might be to just copy the new files (and the ones you modified) to your forked repo, make some new commits there, and then push them back to Github.

A bigger question here (that should maybe have a new issue to discuss), and that @dankelley likely has some opinions on, is how best to integrate functions into GSW-R that don't use the GSW-C library as a source. Currently everything in GSW-R is simply a wrapper around the C library. While this limits GSW-R somewhat, because not all the Matlab functions are available in C, it makes the package MUCH easier to maintain because we can do an update simply by incorporating the new C code when it updates (which is itself updated from the Matlab code). As far as I know the GSW-Python package works the same way.

As an aside, while it's not the same as the gsw_distance function, have you tried using geodDist() function in the oce package? It actually uses an ellipsoidal formula, which should be more accurate than the gsw_distance spherical one. Note even the help for gsw_distance says that:

Note. Distances are probably good to better than 1% of the "true" distance on the ellipsoidal earth.

[chrisdane]

Hey Clark

I managed to transfer all changes from repoA to repoB via

$ cd repoA
$ git stash
$ git stash show -p > patch
$ cp patch ../repoB/
$ cd ../repoB
$ git apply patch

and now wait for an answer to this GSW-Matlab issue (maybe you have an idea?).

Regarding the wrapper around the C-library, would it then make sense to have an additional src/gsw_oceanographic_toolbox.c or similar, where we can add additional functions which are not included in the the GSW-C package?

Regarding gsw_distance, I was interested to see the influence of the depth (or height) of the locations on the distances. As far as I understand, oce::geodDist does not take the depth/height of the locations into account.

Chris

[dankelley]

Correct, oce::geodDist is along the surface, i.e. where ships are. Since ocean depth is seldom more than 6km and earth radius is is order 6000 km, the error ought to be under 0.1 percent. I think the formula, which approximates an integral with an interative scheme, is good to order centimetre over ocean-width scales. There is a citation in the manpage for that function that has more details.

[dankelley]

I'm adding a new issue, and I see that this ought to have been closed long ago, given the age of the last comment. So, closing now.