EcoJulia / RasterDataSources.jl

Easily download and use raster data sets in Julia
MIT License
21 stars 10 forks source link

add CHELSA V2 #62

Closed tiemvanderdeure closed 1 month ago

tiemvanderdeure commented 5 months ago

This package currently downloads CHELSA data from V1.

CHELSA V2 has a bunch of additional variables, so I want to make those accessible from this package.

I think version 2 should become the default, but we need to still allow for version 1. The easiest way to do that would be to have version as a keyword argument for the CHELSA type

To-do list

rafaqz commented 5 months ago

Sure Chelsa version type parameter makes sense, 2 is assumed if not specified

rafaqz commented 5 months ago

Re test failures: the Aqua.jl keywords have changed. We'll need to remove the toml formatting part and also probably specify the Aqua.jl version in Project.toml

tiemvanderdeure commented 4 months ago

Just need to add some tests and this is good to go.

For future climate, I haven't added a version keyword because CMIP6 is always version 2.

The bioclim variables are in the exact same path as the bioclim+ variables, so we could consider making it so that getraster(CHELSA{BioClim}) calls getraster(CHELSA{BioClimPlus}), and that will save us some lines of code.

tiemvanderdeure commented 3 months ago

@rafaqz could you review this and help me troubleshoot these test failures?

rafaqz commented 3 months ago

Ok so Aqua.jl dosen't do toml formatting anymore, yoy can delete that line from the tests.

rafaqz commented 3 months ago

Aqua is in the runtest.jl file, near the top. While youre at it maybe add a line there to include("chelsa_climate.jl") ;)

tiemvanderdeure commented 3 months ago

@rafaqz It now fails because of codecov. Could you take a look?

rafaqz commented 3 months ago

This looks good to go. Do you want to also update the readme table so CHELSA includes plus and climate?

(We can ignore that error its just codecov)

tiemvanderdeure commented 2 months ago

FYI @rafaqz - precompilation gives this output. I can't figure out which line triggers this (nothing to do with this PR I think - it's the same on master)

Precompiling RasterDataSources
        Info Given RasterDataSources was explicitly requested, output will be shown live
┌ Warning: Replacing docs for `RasterDataSources.getraster :: Union{}` in module `RasterDataSources`   
└ @ Base.Docs docs\Docs.jl:243
rafaqz commented 2 months ago

Yeah would be nice to get a line number for that...

Fixed in #65

rafaqz commented 2 months ago

How can WorldClim downloads be this unreliable

tiemvanderdeure commented 1 month ago

Are you kidding me, worldclim is down again!! I think this PR is done - can you re-trigger tests manually when worldclim is up again @rafaqz?

rafaqz commented 1 month ago

Yeah, maybe we need to cut any testing dependency on them or make it optional based on if it loads at all... Or maybe we can use GitHub actions cache somehow to cache the downloads now

It's so weird for such a widely used dataset.

tiemvanderdeure commented 1 month ago

@rafaqz I think this PR is good to go - the tests just failed because worldclim was down. Can you manually re-run the tests? And maybe give the code a final glance?

rafaqz commented 1 month ago

Thanks, great to have this :)