Closed VeruGHub closed 2 years ago
Mathias Newmann suggests also this improvement
I think we could add a new function to the package wrapping get_daily_climate
but allowing for more than one climatic variable, only if the output is a dataframe (returning an error if asking for output raster with >1 climatic var)
Basically we would call get_daily_climate
twice or thrice and then bind the columns of the obtained dataframes.
Would you like to give it a try @VeruGHub ?
I'm thinking this wrapper function should in fact be called get_daily_climate
(ie allowing for >1 climatic var when output = df), so we should then rename the current get_daily_climate
to get_daily_climate_single
or something like that
I'm working on this. @Pakillo @Julenasti what do you think about allowing more than one climatic variable if the output is "raster" and give a list of rasters as output?
I discuss with Julen the possibility of creating a SpatRaster with layers for each variable, but I am not convinced since the SpatRaster of each variable already has many layers (one per day downloaded).
For now, a list of rasters is given after running get_daily_climate with output = "raster" and multiple climatic variable. I can change it in the future if necessary
Great job @VeruGHub! I see your point. My first answer will be to use a SpatRaster since you already work with this type of objects but you're right that it already has many layers. So I don't know a better solution than a list... I'll check it out this days!
Thanks Vero for pushing this.
I don't think raster layers from different climatic variables should ever be combined in a single SpatRaster object. Imagine the mess when layers 1 to 23 represent precipitation, 24 to 46 tmin, etc. Different units, different variables, etc.
Of course, any user could combine the layers in a single raster if they wish. But I don't think we should do it.
I'm fine with a named list of rasters if you wish, as currently implemented. If you ask for a single climatic var do you get a list as well, or a SpatRaster?
I see get_daily_climate now always returns a list of rasters, even if asking for a single climatic variable. It's bad for consistency of output types, but I think it's much more intuitive to return a raster rather than a list in those cases (ie only one climatic var). Otherwise the user always has to unlist somehow...
Hi! Ok, then, let's keep the named list when output is "raster" and the user wants more than one climatic variable. I have to take a look because get_daily_climate should be given the same than get_daily_climate_single when climatic_var length is 1, and a named list otherwise. Tests were working but maybe a I did a mistake in the tests.
Sorry Vero, you're right!! With only one climatic_var you still get a raster
Totally fine then!
Yes, we need to revise tests, as checks are failing
No problem :) I ran the the tests in my computer and everything was working. However, when I did the commit the checks kept failing... I have to take a look since I thought the reason was a problem with the connection to the server that day but I am not completely sure.
Yes, it seems the problem is with access to the FTP server (error 530). I've disabled the tests with multiple climatic vars by now. It runs fine in my desktop too
Closing, as the feature has been successfully implemented. Thanks Vero!
It can be easy to implement and useful when the output is a data.frame - the user would get a data frame with each climatic variable in one column. When the output is a raster, it can be more messy to mix variables. @Julenasti any proposal in this sense?
In any case, the solution is to call twice or three times to the server, because each variable is stored in a different file. We could do it internally or the user could use get_daily_climate twice or three times and both solutions are user-friendly :)