search

ropensci / rnoaa

R interface to many NOAA data APIs
https://docs.ropensci.org/rnoaa
Other
328 stars 84 forks source link

Vectorises ghcn #356

Closed eliocamp closed 3 years ago

eliocamp commented 4 years ago

I thought it would be useful if the function ghcn() could accept a vector of station ids.

sckott commented 4 years ago

perhaps. ghcnd() is used within ghcnd_search(), which is used within meteo_tidy_ghcnd(). So this change will have to be checked carefully that it doesn't break those other 2 functions

eliocamp commented 4 years ago

I checked and both meteo_tidy_ghncd() and ghcnd_search() work with length one arguments and vector arguments (i.e. they inherit vectorisation for free :money_mouth_face: )

There is the issue of duplicated entries which arise if the user inserts duplicated ids. I don't know that would it be best to do in that case. I guess filter out the duplicates and run the functions with just the unique values?

EDIT: Had run the wrong test.

sckott commented 4 years ago

Thanks. Can you run any tests for ghcnd_search/meteo_tidy_ghncd and make sure they still work - and add some new tests for ghncd the vectorization

sckott commented 3 years ago

@eliocamp still plan to finish this? if not, lets close this

eliocamp commented 3 years ago

Ouch, sorry! It totally fell off my plate. I'll get to it this weekend. Apologies!

eliocamp commented 3 years ago

There! The relevant tests run well on my machine. I tried to test the whole package to see if I accidentally broke something else (not likely, since the changes are well contained to one function) but I got too many failures related to missing API keys.

sckott commented 3 years ago

Thanks @eliocamp ! Can you send a new pull request on a branch with a different name? There's so many files changed in this PR it makes it hard to know what's going on