search

International-Soil-Radiocarbon-Database / ISRaD

Repository for the development and release of ISRaD data and tools
https://international-soil-radiocarbon-database.github.io/ISRaD/
24 stars 15 forks source link

Package to CRAN #190

Closed jb388 closed 4 years ago

jb388 commented 5 years ago

@greymonroe @aahoyt @coreylawrence @crlsierra @alkalifly @ShaneStoner To do:

jb388 commented 5 years ago

Sorry for the spam on this issue, but the data, in fact, IS still included in the package right now.

So, I think once it's been pulled, we should be able to submit to CRAN, right?

greymonroe commented 5 years ago

Yeah it should be ready for cran. I am submitting it this weekend

On Jun 20, 2019, at 6:58 PM, Jeff B notifications@github.com wrote:

Sorry for the spam on this issue, but the data, in fact, IS still included in the package right now.

So, I think once it's been pulled, we should be able to submit to CRAN, right?

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

greymonroe commented 5 years ago

also there is the graven.rda object in the data folder. Sorry if im just forgetting but when did this get added? Cant seem to recall the history of this file

jb388 commented 5 years ago

I think you added that file. Since that file shouldn't change with any frequency, I think it's fine to keep that in the package.

greymonroe commented 5 years ago

Haha ok that’s what I thought but when I looked at commit history it seemed to show it was only 2 days old

On Jun 22, 2019, at 11:47 AM, Jeff B notifications@github.com wrote:

I think you added that file. Since that file shouldn't change with any frequency, I think it's fine to keep that in the package.

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

jb388 commented 5 years ago

Yeah, when I changed the name I had to replace the object.

greymonroe commented 5 years ago

https://builder.r-hub.io/status/ISRaD_0.1.1.tar.gz-33874d571ec84bd1a505ee2a82956b2e

We may be having some issues sending to CRAN. I think we have too many package dependencies

jb388 commented 5 years ago

Hmm, thanks for checking this, @greymonroe

These are the errors I found in the builder output you linked:

3404#> installing source* package ‘htmlwidgets’ ... 3410#> Failed with error: ‘there is no package called ‘shiny’’ 3490#> ERROR: configuration failed for package ‘openssl’ 3851#> ERROR: dependency ‘openssl’ is not available for package ‘httr’ 3898#> ERROR: dependency ‘httr’ is not available for package ‘gh’ 3967#> ERROR: dependency ‘httr’ is not available for package ‘rvest’ 4124#> ERROR: dependency ‘httr’ is not available for package ‘oai’ 4198#> ERROR: dependency ‘gh’ is not available for package ‘usethis’ 4232#> ERROR: dependencies ‘usethis’, ‘httr’ are not available for package ‘devtools’ 4300#> ERROR: dependency ‘oai’ is not available for package ‘pangaear’ 4397#> ERROR: dependency ‘httr’ is not available for package ‘ggmap’ 4432#> ERROR: dependencies ‘httr’, ‘rvest’ are not available for package ‘tidyverse’ 4447#> ERROR: dependencies ‘devtools’, ‘ggmap’, ‘pangaear’, ‘tidyverse’, ‘usethis’ are not available for package ‘ISRaD’

I think we definitely have too many package dependencies (131!), but are you sure that's what is causing the build failure? Seems like it may be a few specific packages that are failing for whatever reason and the errors are cascading through all the packages that depend on those failed packages.

Anyway, I'll try and take a look at this this week, probably Tuesday or Wednesday.

greymonroe commented 5 years ago

Ok thank you. Yes, perhaps we can just trim out the unnecessary stuff. In fact, I would be ok removing the shiny app entirely. What do you think?

On Jul 28, 2019, at 7:03 PM, Jeff B notifications@github.com wrote:

Hmm, thanks for checking this, @greymonroe https://github.com/greymonroe These are the errors I found in the builder output you linked:

3404#> * installing source package ‘htmlwidgets’ ... 3410#> Failed with error: ‘there is no package called ‘shiny’’ 3490#> ERROR: configuration failed for package ‘openssl’ 3851#> ERROR: dependency ‘openssl’ is not available for package ‘httr’ 3898#> ERROR: dependency ‘httr’ is not available for package ‘gh’ 3967#> ERROR: dependency ‘httr’ is not available for package ‘rvest’ 4124#> ERROR: dependency ‘httr’ is not available for package ‘oai’ 4198#> ERROR: dependency ‘gh’ is not available for package ‘usethis’ 4232#> ERROR: dependencies ‘usethis’, ‘httr’ are not available for package ‘devtools’ 4300#> ERROR: dependency ‘oai’ is not available for package ‘pangaear’ 4397#> ERROR: dependency ‘httr’ is not available for package ‘ggmap’ 4432#> ERROR: dependencies ‘httr’, ‘rvest’ are not available for package ‘tidyverse’ 4447#> ERROR: dependencies ‘devtools’, ‘ggmap’, ‘pangaear’, ‘tidyverse’, ‘usethis’ are not available for package ‘ISRaD’

I think we definitely have too many package dependencies (131!), but are you sure that's what is causing the build failure? Seems like it may be a few specific packages that are failing for whatever reason and the errors are cascading through all the packages that depend on those failed packages.

Anyway, I'll try and take a look at this this week, probably Tuesday or Wednesday.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/International-Soil-Radiocarbon-Database/ISRaD/issues/190?email_source=notifications&email_token=AD7HB7HEDOFYFLDC5FOFUXDQBXGM3A5CNFSM4HX2BFO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD27CQNI#issuecomment-515778613, or mute the thread https://github.com/notifications/unsubscribe-auth/AD7HB7GOFPYHWCAGKHFSYIDQBXGM3ANCNFSM4HX2BFOQ.

jb388 commented 5 years ago

Sure, I don't think anyone is really using the shiny app any more. However we do describe it in the manuscript (ln 342:345), but we could just delete that paragraph. @coreylawrence any concerns with axing shiny?

jb388 commented 5 years ago

@greymonroe Unfortunately I'm not making much progress with this. I'm not at all sure it will solve the problem, but maybe cutting shiny is a good idea. Seems possible it is the source of the problem---it's the only package that has the "failed" message---but I'm not 100% convinced.

Interestingly, we don't actually rely directly on that many packages. In fact, there are only 10 that we call explicitly (if we don't include all of the packages that are included within tidyverse, i.e. dplyr, tidyr, ggplot2, stringr, or the ones included with every R installation, i.e. base, utils, stats). However, many of those 10 packages have a ton of dependencies. The point is that I'm not sure what we can cut, other than shiny.

The Description file lists more dependencies than 10, but that's due to redundancies of tidyverse packages and the inclusion of two we don't actually use: e.g. I couldn't find where we call 'usethis' in any of the functions, nor 'plyr'.

jb388 commented 5 years ago

Also, the builder link isn't working any longer. I tried to upload the tar.gz file, but I think since you are the maintainer, @greymonroe , you are the only one who can run it.

greymonroe commented 5 years ago

Hmm ok. Yes I think a thorough review of package dependencies could be warranted. Let’s first try to remove shiny and see if that solves it. Jeff do you want to go over that together so you can see the cran submission process?

Not sure what you mean by builder. Can you explain?

On Aug 5, 2019, at 10:08 PM, Jeff B notifications@github.com wrote:

Also, the builder link isn't working any longer. I tried to upload the tar.gz file, but I think since you are the maintainer, @greymonroe , you are the only one who can run it.

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

jb388 commented 5 years ago

@greymonroe what I meant by builder was just this link: https://builder.r-hub.io/status/ISRaD_0.1.1.tar.gz-33874d571ec84bd1a505ee2a82956b2e

jb388 commented 5 years ago

Sure, I'd be interested to learn more about the CRAN submission process.

coreylawrence commented 5 years ago

Sorry to be late to the party. I am totally fine with axing the Shiny interface. Since it isn't really served through the website, the intended utility isn't there anyways. We could always try to bring it back for the website at some point in the future (assuming we can find a good way around the hosting issue) but I don't see any need to include it in the R-package.

jb388 commented 5 years ago

No worries, @coreylawrence. Definitely been to more fun parties. Thanks for weighing in. Let's go ahead and cut shiny; hopefully that will fix the problem.

greymonroe commented 5 years ago

Ok I’ll get going on this

On Aug 6, 2019, at 6:08 AM, Jeff B notifications@github.com wrote:

No worries, @coreylawrence. Definitely been to more fun parties. Thanks for weighing in. Let's go ahead and cut shiny; hopefully that will fix the problem.

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

greymonroe commented 5 years ago

Resubmitted to CRAN. fingers crossed.

@jb388 Here is the code with the steps to release (or update) the package. 

# rebuild to make sure everything looks good
ISRaD.build(geodata_clim_directory ="~/Seafile/ISRaD_keeper/ISRaD_extra_spatialdata/", geodata_soil_directory="~/Seafile/ISRaD_keeper/ISRaD_extra_spatialdata/ISRaD_extra_soiltax", geodata_pet_directory = "~/Seafile/ISRaD_keeper/ISRaD_extra_spatialdata/pet/" )

# update minor version number 
ISRaD_directory<-getwd()
DESC<-readLines(paste0(ISRaD_directory,"/DESCRIPTION"))
version<-unlist(strsplit(DESC[3],split = "\\."))
version<-version[1:3]
version[3]<-as.numeric(version[3])+1
DESC[3]<-paste(unlist(version), collapse = ".")
writeLines(DESC, paste0(ISRaD_directory,"/DESCRIPTION"))

# the release() function will ask that you've done this so you might as well run now
devtools::document()
devtools::spell_check()
devtools::check()
devtools::check_rhub()
devtools::check_win_devel()

# this is to release to CRAN
devtools::release()

BTW do you want to become the official maintainer?

jb388 commented 5 years ago

@greymonroe OK, thanks. Sure---I can take over official maintainer status.

jb388 commented 5 years ago

@coreylawrence @aahoyt @ShaneStoner @greymonroe I pulled the problematic vignette (explore-fractions.rmd), switched the maintainer role from Grey to me, and rebuilt ISRaD for a new submission. But apparently the CRAN team is on vacation from August 9th til August 18th. No CRAN submissions possible during this time. I'll resubmit on the 18th and keep you posted.

jb388 commented 4 years ago

@greymonroe I know you've passed on the title of maintainer, but I'm still running into this issue with the openssl configuration. The build works on the following platforms:

I'm getting the same error as before (see report here https://builder.r-hub.io/status/ISRaD_0.1.4.tar.gz-e86d06be0e4a407787381ecf556c8ec1):

4207#> ------------------------- ANTICONF ERROR --------------------------- 4208#> Configuration failed because openssl was not found. Try installing: 4209#> deb: libssl-dev (Debian, Ubuntu, etc) 4210#> rpm: openssl-devel (Fedora, CentOS, RHEL) 4211#> csw: libssl_dev (Solaris) 4212#> brew: openssl@1.1 (Mac OSX) 4213#> If openssl is already installed, check that 'pkg-config' is in your 4214#> PATH and PKG_CONFIG_PATH contains a openssl.pc file. If pkg-config 4215#> is unavailable you can set INCLUDE_DIR and LIB_DIR manually via: 4216#> R CMD INSTALL --configure-vars='INCLUDE_DIR=... LIB_DIR=...'

I don't know how to direct R to look at the proper configuration file when building on a remote server via docker.

greymonroe commented 4 years ago

Hmm could be part of a general issue with openssl

https://github.com/swirldev/swirl/issues/513 https://github.com/swirldev/swirl/issues/513 https://github.com/rocker-org/rocker/issues/124 https://github.com/rocker-org/rocker/issues/124

Can you see which packages we require depend on openssl?

On Sep 9, 2019, at 10:32 PM, Jeff B notifications@github.com wrote:

@greymonroe https://github.com/greymonroe I know you've passed on the title of maintainer, but I'm still running into this issue with the openssl configuration. The build works on the following platforms:

my local machine (mac 10.13.4) Windows Server 2008 R2 SP1, R-devel, 32/64 bit Ubuntu Linux 16.04 LTS, R-release, GCC but it DOES NOT WORK for Fedora Linux, R-devel, clang, gfortran I'm getting the same error as before (see report here https://builder.r-hub.io/status/ISRaD_0.1.4.tar.gz-e86d06be0e4a407787381ecf556c8ec1 https://builder.r-hub.io/status/ISRaD_0.1.4.tar.gz-e86d06be0e4a407787381ecf556c8ec1):

4207#> ------------------------- ANTICONF ERROR --------------------------- 4208#> Configuration failed because openssl was not found. Try installing: 4209#> deb: libssl-dev (Debian, Ubuntu, etc) 4210#> rpm: openssl-devel (Fedora, CentOS, RHEL) 4211#> csw: libssl_dev (Solaris) 4212#> brew: openssl@1.1 (Mac OSX) 4213#> If openssl is already installed, check that 'pkg-config' is in your 4214#> PATH and PKG_CONFIG_PATH contains a openssl.pc file. If pkg-config 4215#> is unavailable you can set INCLUDE_DIR and LIB_DIR manually via: 4216#> R CMD INSTALL --configure-vars='INCLUDE_DIR=... LIB_DIR=...'

I don't know how to direct R to look at the proper configuration file when building on a remote server via docker.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/International-Soil-Radiocarbon-Database/ISRaD/issues/190?email_source=notifications&email_token=AD7HB7EA3T3JF4WMO34FT43QI2XEXA5CNFSM4HX2BFO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6I6ALQ#issuecomment-529653806, or mute the thread https://github.com/notifications/unsubscribe-auth/AD7HB7FBU3DFTATZCQZXV5TQI2XEXANCNFSM4HX2BFOQ.

jb388 commented 4 years ago

@greymonroe Pretty sure everything tidyverse depends on openssl in some recursive fashion.

greymonroe commented 4 years ago

Bummer. Really have no idea what this error is about. I would be shocked if every package that used tudyverse failed submission. You could always go ahead and try to submit anyway

On Sep 10, 2019, at 9:48 AM, Jeff B notifications@github.com wrote:

@greymonroe Pretty sure everything tidyverse depends on openssl in some recursive fashion.

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

jb388 commented 4 years ago

Bummer. Really have no idea what this error is about. I would be shocked if every package that used tudyverse failed submission. You could always go ahead and try to submit anyway …

Right? I feel like I'm missing something simple here, or it should be easier to find a solution given the ubiquity of tidyverse dependencies. But it's hard to troubleshoot the issue since it only occurs with an install on a completely different system. I went ahead and submitted it to CRAN anyway, since it did pass all the checks on the other systems. So fingers crossed...

jb388 commented 4 years ago

Fixed the Fedora platform config error. Had to change the PKG_CONFIG_PATH by installing openssl@1.1 w/ homebrew. Just as the error message suggested, hah. Package now ready for CRAN, just waiting on final edits/documentation of updated geospatial fxs.

jb388 commented 4 years ago

And...we're finally on CRAN!