search

JGCRI / fldgen

Given a global mean temperature pathway, generate random global climate fields consistent with it and with spatial and temporal correlation derived from an ESM
https://jgcri.github.io/fldgen/
GNU General Public License v2.0
12 stars 6 forks source link

Problem with testing fldgen on developmental version of R #42

Closed kdorheim closed 4 years ago

kdorheim commented 4 years ago

We were originally using github actions to test fldgen on across platforms and on two versions of R, the current version (3.6) and the developmental version. But our dependency on ncdf4 breaks the package on dev R. As of 2020-04-30 (see 00239e0) we've removed this test but ensuring that fldgen can run on newer versions of R will be an issue to address in the future.

bpbond commented 4 years ago

Is ncdf4 just not available yet for dev-r? I.e., is this a temporary, or potentially permanent, issue?

kdorheim commented 4 years ago

I would hope that ncdf4 is just not available yet, I can't imagine they wouldn't update that package... however it is not that would be a major issue for fldgen and several other projects at the institute

abigailsnyder commented 4 years ago

yeah, hopefully temporary. As far as i know, ncdf4 is one of the most widely used packages for using netcdfs in R. I cannot imagine it not being maintained and updated, hopefully in the not too distant future.

kdorheim commented 4 years ago

yea hopefully we can add the dev test back soon because it catches things like this but if we are going to make the github actions tests requirements to merge PR I didn't want that to prevent PRs from being merged. @abigailsnyder if it looks like you can select specific github action tests as must pass to merge while excluding others then we could add it back in 🤷‍♀️

abigailsnyder commented 4 years ago

@kdorheim done - we now block PRs from merging if they fail any test except the developmental version of R. So as you say, it's good to have it added back in because it lets us know what problems may be coming our way. So this way a PR can fail that test to warn us, but still be ok to merge if all the other tests pass.

kdorheim commented 4 years ago

Looks like @abigailsnyder added specific PR merge requirements so can add the developmental version of R as a github actions test but that is low priority for the moment

bpbond commented 4 years ago

Nice 👏

kdorheim commented 4 years ago

This test as been re-added because it will help us identify potential decency problems on the horizon.