search

BiologicalRecordsCentre / sparta

Species Presence/Absence R Trends Analyses
http://biologicalrecordscentre.github.io/sparta/index.html
MIT License
21 stars 24 forks source link

fixing error arising from changes in how R stores package version information #256

Closed drnickisaac closed 2 months ago

drnickisaac commented 3 months ago

current version of sparta fails with new version of R. This should fix it.

AugustT commented 3 months ago

@DylanCarbone can you take a look at this. I see checks have failed but I think this is the known R2Jags installation issue

DylanCarbone commented 3 months ago

Hi @AugustT

Sure, I'll take some time this week to look at this! This function requires both JAGS and R2Jags installations. My current understanding is that R2Jags and other package dependencies can be installed from cran or github in the R-CMD check. However, I am struggling to find a method of installing JAGS itself.

As a plan B, I've been configuring tests to skip when there is no JAGS installation. One example is here. I'll also configure the R-CMD check to ignore compiling the vignette.

I'm slowly creating new tests and a new R-CMD check, moving towards Github actions with usethis. But this will take some time, and I'm starting with the BRCindicators package. We can talk about my progress more during Thursdays meeting,

AugustT commented 3 months ago

Thanks @DylanCarbone this sounds like a good direction to be moving in. In the meantime, to get this specific issue resolved, could you download and test this version to your machine and test it locally, where JAGS should not cause an error? Or maybe fork Nicks repo and test that. Alternatively, @drnickisaac if you have already run the tests locally and can confirm it has past all tests and checks that would suffice.

DylanCarbone commented 3 months ago

Hi @AugustT and @drnickisaac,

I've just cloned Nick's Repo and tested for function test failures. The OccDetFunc.r tests are successful, but there are failures in some of the function dependency tests. I can take a look at this today.

DylanCarbone commented 3 months ago

Hi @AugustT and @drnickisaac,

Sorry that this took me longer than expected.

I took a longer look today, and I was mistaken, there were no errors within any of the function dependencies. However, the test outputs of the function and it's dependencies is challenging to interpret because the tests are so long, incorporating multiple error checks. The recommendation is that each test should have closer to one error check (or other check of an output). I've began to tweak the helper functions to meet the recommendation, see here. I've also created short helper files to create helper datasets here. However I still need to work on the occDetFunc.r tests which are very long.

I've configured all tests used in the occDetFunc function and it's dependancies to skip if there is no JAGS installation, so there should be no errors raised for the functions use by Appveyor. However, I need to transition the package away from Appveyor - it has very little recent documentation, and I am struggling to configure it to work on my forked repositories.

If you don't mind Nick, I will close your pull request and replace it with my own, incorporating your changes.

Thanks,

Dylan