fix #133

[mpadge]

This should fix #133. Changes were manifold, due to both tibble updates breaking some of the sub-settting, and rvest updates which greatly improved extraction of the messy tables, so former assumptions on structure of return values no longer applied.

The PR happily removes most of the formerly messy code individually addressing individual rows and columns of the tables, and reduces the tidy_bulletin_header fn down to two simple sub-functions. Much nicer!

Confirmation that all works:

setwd ("/data/mega/code/forks/bomrang")
devtools::load_all (".", export_all = FALSE)
#> ℹ Loading bomrang
#> Registered S3 method overwritten by 'hoardr':
#>   method           from
#>   print.cache_info httr

states <- c ("vic", "nsw", "qld", "nt", "tas", "wa", "sa")
vapply (states, function (i)
        nrow (get_weather_bulletin (i, morning = TRUE)),
        integer (1))
#> vic nsw qld  nt tas  wa  sa 
#>  93 188 134  56  49 136  90
vapply (states, function (i)
        nrow (get_weather_bulletin (i, morning = FALSE)),
        integer (1))
#> vic nsw qld  nt tas  wa  sa 
#>  82 140 132  53  39 132  71

^{Created on 2021-03-25 by the reprex package (v1.0.0)}

[adamhsparks]

Thanks, @mpadge. I poked at it a little bit yesterday but wasn't familiar enough to make these changes so quickly!

I'm working on it now, modifying the function slightly to use curl_download() to take advantage of curl's options with the custom handle since I was getting a timeout with this function and I've updated the test to check the new colnames.

[mpadge]

Have you tried using crul for async requests? I suspect that might be more straightforward than processing curl_download results when things go wrong? Just a thought in case that helps

[adamhsparks]

Forgive my ignorance here, I've used crul where I actually have a proper API to talk to, nasapower, but I don't understand what you're fixing by using it here?

[mpadge]

Forgive my ignorance here, I've used crul where I actually have a proper API to talk to, nasapower, but I don't understand what you're fixing by using it here?

Sorry, only because you mentioned timeout issues which it can help with. But I'm sure you've got a better idea than me here, so please ignore and continue as you were, good sir!

[adamhsparks]

Thanks, Mark. I have to confess, crul does more than I fully understand, so I thought maybe you were onto something here.

On 26 Mar 2021, at 8:40 pm, mark padgham @.***> wrote:

 Forgive my ignorance here, I've used crul where I actually have a proper API to talk to, nasapower, but I don't understand what you're fixing by using it here?

Sorry, only because you mentioned timeout issues which it can help with. But I'm sure you've got a better idea than me here, so please ignore and continue as you were, good sir!

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or unsubscribe.

[adamhsparks]

I merged this locally with the devel branch. Thanks again for the quick response, @mpadge.