Archival of rnbn

[AugustT]

This issue will be kept up to date with news regarding the deprecation of the rnbn. The APIs will move over to the Atlas of Living Australia architecture and a new R package will be developed.

Current status

• rnbn is now OFFLINE
• A replacement package is in development and is available here: https://github.com/BiologicalRecordsCentre/NBN4R
• rnbn can still be accessed on the CRAN archives but will not function properly: https://cran.r-project.org/src/contrib/Archive/rnbn/rnbn_1.1.2.tar.gz

[sckott]

@AugustT can you ask CRAN to archive the pkg please

[AugustT]

Archived and info above updated

[sckott]

great, thanks so much

[fozy81]

@AugustT I've forked ALA4R and updated to link to nbn api: https://github.com/fozy81/ALA4R
Appears to be working okay, but the documents and vignettes etc will need some updating. Any thoughts on how best proceed?

[raymondben]

Chaps - can I be so bold as to suggest a different approach here? With this package forked from ALA4R we'll effectively have two independent packages that will diverge over time, and it will probably be difficult to propagate ALA4R improvements into yours and vice-versa.

Instead, I suspect it would be better for you to create a new package that Imports: ALA4R. In that package's .onLoad function you can override the ALA4R-specific settings:

.onLoad <- function(libname,pkgname) {
    temp <- getOption("ALA4R_server_config")
    temp$brand <- "ALUK" ## or "rnbn" or whatever you're calling it
    ## override any other settings here
    options(ALA4R_server_config=temp)
}

Then for the functions you want to expose through your package, write wrappers, e.g.:

#' updated docs here
species_info <- function(...) ALA4R::species_info(...)

You can provide your own documentation (e.g. with rnbn-specific links, instead of the ALA ones) and also modify function behaviour if needed by changing inputs and outputs to the underlying ALA4R function. You can also provide your own vignettes etc.

With this approach, any changes to ALA4R will propagate through nicely to yours, and if you make improvements then both packages benefit.

[AugustT]

Hey Tim,

Thanks for taking this forward, good to see someone is making progress! Ben and I had a chat several months ago and decided that the approach he outlined was the best way forward. Sorry you are only learning that now. As Ben says there are many benefits to this approach, it probably would not take you much time an effort to change you fork into this wrapper package.

Let me know how you get on.

[fozy81]

@raymondben @AugustT That sounds a good approach, thanks for the outlined example 👍 . I'll make a start - happy for anyone to pick it up/move to a more permanent home, just thought I'd give it a try. It was really easy to switch urls based on the work done on ALA4R. So best to stay in sync as far as possible. Also crossed my mind about taxize package and linking into that.

[fozy81]

@AugustT @raymondben I've updated the package making it wrap all the ALA4R functions. I've kept in all the internal functions for now but these can be deleted. The working name is NBN4R but maybe best to continue with rnbn or something else?

Anyway, the easy bit is done, now need to update docs - lots of 'penguin' and 'kangaroo' examples need updating! 😄

[AugustT]

Great stuff Tim. Can you remind me who you work for? I think NBN4R is a good name. We don't want to use rnbn because people will confuse it with the old package which worked in a very different way.

Since you have made a start on this shall we create a home for it where are can all chip in as needed and we can move this discussion from this thread? I dont mind where, as long as it is open. We can keep it in your repo Tim if you like? Perhaps we can start a new thread there to carry on this conversation?

[fozy81]

Hi Tom, I work for SEPA, we met briefly at the SNH tech conference. This is my personal account just for tinkering. Wanted to get some practice at R packages especially api wrappers so this piqued my
interest. It's also could be handy for some professional bits and pieces.

Happy for it to sit on ropensci or BRC? As I'm only doing this in free time and may not be around to maintain etc.

[AugustT]

Okay. I'm off for a months holiday, when I get back I will see about creating a copy on the BRC repo

[fozy81]

Great, enjoy your holiday. 👍

[raymondben]

Hmm, sorry, think I've given you some slightly misleading advice: if ALA4R functions are wrapped just with dots:
NBN4Rfunc <- ALA4R::ALA4R_func(...)
then check will fail because the documentation will have the actual function parameters whereas check will be expecting only dots to be documented.
One way around this is to have the NBN4R version of the function declare the full argument list from the ALA4R function that is is wrapping, e.g.

species_info <- function(scientificname,guid,verbose=nbn_config()$verbose) 
    ALA4R::species_info(scientificname,guid,verbose=nbn_config()$verbose)

(assuming you'll have nbn_config as a wrapper around ala_config)
The drawback to that is that if the underlying ALA4R function argument list changes, that change will have to be manually propagated to the NBN4R wrapper function. It might be easy to miss these changes, so it would be worth including a test that compares argument lists between each function pair.

I don't know if there's a better way to do it. A straight re-export of the ALA4R functions won't work, because we want to change function names and documentation. - @sckott, any suggestions?

[fozy81]

Thanks Ben, I understand the issue, I'll try to update in the next few days and add a test to check the functions arguments are in sync.

[raymondben]

I think the testing is fairly straightforward, something like
expect_named(formals(nbn_function),names(formals(ALA4R::ala_function)),ignore.order=TRUE)
ought to do it.

[fozy81]

It could be worth thinking about testing reverse dependency in ALA4R. Maybe just a warnings so folks updating ALA4R are aware of impacts on NBN4R (and perhaps other dependent packages) and can warn them. So we catch issues on both sides. But depends how tightly you want to couple the packages.

[raymondben]

Good idea, I reckon. The ALA server infrastructure is being used by a number of national installations (UK, Sweden, France, Costa Rica, ...) so it's possible/likely that there will be other wrapper packages like NBN4R. ALA4R tests already do some testing hitting a French or Spanish server - but it's pretty minimal thus far.

[fozy81]

Hi, I've wrapped all the arguments and added tests to check arguments match. I've moved it to its own repo fozy81/NBN4R. Adding CI etc. I've made a draft update of the readme (based on ALA4R) but not the vignette. Skipped a few tests...need closer look at these.

R CMD check results
0 errors | 0 warnings | 0 notes 🎆 🍾

Probably won't be working on this again for a while. But feel free to PR/fork/issues etc and we can pick up again when Tom is back.

[fozy81]

@AugustT @raymondben any thoughts on the best place for this to resided? I've started using a little for some invasive species analysis. There still some work to do in terms of vignette, broken tests and dependencies checking. I've had fun getting it working (ish), it's been useful and happy to help maintain but not sure I'm best placed to host and keep it up to date. Might be good to discuss offline sometime. Kind regards.

[AugustT]

@fozy81 sorry I forgot about this, I've now forked this to the BRC's repo (https://github.com/BiologicalRecordsCentre/NBN4R) and added you as a collaborator with admin privileges (@raymondben I added you as a regular collaborator). It looks like you have made good progress, and I'm happy to put in some work to tidy things up, though this is unlikely to be in the near future. I'm still chasing NBN to see if I can get support for this...

[AugustT]

Please continue discussion of NBN4R here: BiologicalRecordsCentre/NBN4R#1

[maelle]

Should this repo be archived after getting an unsupported repostatus.org badge and a link to the newer package?

[AugustT]

Sounds good to me. Its strange that repostatus.org does not have an achieved badge.

[maelle]

I agree. Can you make the commit and then I'll archive (it'll be read only)? Thanks

[AugustT]

Your wish is my command

[maelle]

Thanks a lot! Am going to archive it.

For the record, since this thread will become read-only. For updates on the new package or to contribute plese find more details at: https://github.com/BiologicalRecordsCentre/NBN4R