search

ropensci-archive / bomrang

:warning: ARCHIVED :warning: Australian government Bureau of Meteorology (BOM) data client for R
Other
109 stars 26 forks source link

Incorporate static images in vignette #88

Closed adamhsparks closed 5 years ago

adamhsparks commented 6 years ago

The automated checks fail as will installation on users' systems if BOM fails to respond with an image.

Suggest incorporating static images into package vignette to avoid errors and make installation faster.

https://cran.r-project.org/web/checks/check_results_bomrang.html

jonocarroll commented 6 years ago

Functions which rely on other servers should probably be robust against incidental failure. A common technique would be retry on fail. As for how to test a function which may fail, I'd like to avoid the popular route of 'just don't test'. With proper error-handling the function could return something of the appropriate class, even if it's empty.

On Mon, 17 Sep. 2018, 1:45 pm Adam H. Sparks, notifications@github.com wrote:

The automated checks fail as will installation on users' systems if BOM fails to respond with an image.

Suggest incorporating static images into package vignette to avoid errors and make installation faster.

https://cran.r-project.org/web/checks/check_results_bomrang.html

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ropensci/bomrang/issues/88, or mute the thread https://github.com/notifications/unsubscribe-auth/AJDpIUROaAE-NK0k8lrdpDbppujbd6b5ks5ucAnpgaJpZM4WsrxR .

adamhsparks commented 6 years ago

Good point. I've implemented a retry where numeric and text data are concerned, but haven't yet with images. Though that seems like a separate issue to me that should be opened as I'd still like the proper image to appear in the vignette.

jonocarroll commented 6 years ago

My issue with static is that it can become stale - either you or BOM change something and all of a sudden the vignette renders but is wrong. Same reason the code is in .Rmd not just .md but as a first-pass mitigation strategy it would work.

On Mon, 17 Sep. 2018, 2:05 pm Adam H. Sparks, notifications@github.com wrote:

Good point. I've done this where numeric and text data are concerned, but haven't yet with images. Though that seems like a separate issue to me that should be opened as I'd still like the proper image to appear in the vignette.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ropensci/bomrang/issues/88#issuecomment-422171216, or mute the thread https://github.com/notifications/unsubscribe-auth/AJDpIaP5Yy5q5511ZOU9JsF-FtgpkjPKks5ucA6qgaJpZM4WsrxR .

adamhsparks commented 6 years ago

OK, I'll agree with that point but what you you put in documentation if BOM doesn't respond at the time the vignette is built?

jonocarroll commented 6 years ago

Warning: failed to retrieve data. This may be intermittent. Please re-build vignette to try again.

On Mon, 17 Sep. 2018, 2:19 pm Adam H. Sparks, notifications@github.com wrote:

OK, I'll agree with that point but what you you put in documentation if BOM doesn't respond at the time the vignette is built?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ropensci/bomrang/issues/88#issuecomment-422175385, or mute the thread https://github.com/notifications/unsubscribe-auth/AJDpISs3_tSbNVh4Bgb72nSeQor7J4J1ks5ucBHfgaJpZM4WsrxR .

adamhsparks commented 6 years ago

As an image file I'm guessing?

jonocarroll commented 6 years ago

Depends how much code you want to write, but that's one option. Another would be to logic-branch the output (possibly with a formal class). Or just catch the error, print it as a message, and return safely from there. If there's no processing of the file downstream from the image that works. If there is, then that needs to know what do do too.

I could imagine some sort of image_or_error class which could be checked during test (being specific about the error).

On Mon, 17 Sep. 2018, 2:25 pm Adam H. Sparks, notifications@github.com wrote:

As an image file?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ropensci/bomrang/issues/88#issuecomment-422177107, or mute the thread https://github.com/notifications/unsubscribe-auth/AJDpIW_sBpmPg7g6n0o9v5ek64Q41eohks5ucBM5gaJpZM4WsrxR .

adamhsparks commented 6 years ago

Nice. Thanks these ideas. I'll see what I can do.

adamhsparks commented 6 years ago

I've not coded anything yet but the .svg and this .png file are now in the devel branch of the repository.

https://github.com/ropensci/bomrang/blob/4ac4345e8ba1c2b533110b1f7aabccdebd0555f0/man/figures/image_error_message.png

adamhsparks commented 5 years ago

I did eventually use a tryCatch() in the functions to point to this error message if the proper file is not downloaded. I just forgot to close this issue.