Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed

Conversation

drnickisaac
Copy link
Contributor

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

@AugustT
Copy link
Member

AugustT commented Mar 17, 2024

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

@DylanCarbone
Copy link
Collaborator

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
Copy link
Member

AugustT commented Mar 19, 2024

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
Copy link
Collaborator

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 DylanCarbone self-assigned this Mar 19, 2024
@DylanCarbone
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants