added cir_img() function

[andschar]

Pull Request

adding the cir_img() function. Replacing the closed, erroneous PR #250. Alreasy uses RETRY() as discussed in PR #271 .

PR task list:

• Update NEWS
• [not appropriate for images?] Add tests (if appropriate)
• Update documentation with devtools::document()
• Check package passed

[Aariq]

Very excited to be adding image functions to webchem! Here are some style suggestions and a couple of issues with the behavior of the function.

[Aariq]

• [not appropriate for images?] Add tests (if appropriate)

I think it might be good to write some tests to check that error messages work as expected and file downloads happen or don't happen when expected. You can use tempdir() to download images inside tests. I don't think it's necessary to check that the correct image is downloaded.

[andschar]

• [not appropriate for images?] Add tests (if appropriate)

I think it might be good to write some tests to check that error messages work as expected and file downloads happen or don't happen when expected. You can use tempdir() to download images inside tests. I don't think it's necessary to check that the correct image is downloaded.

I have added some tests to check the funciton.

[andschar]

@Aariq In the update b988a2b I changed the function back to if statements for all the options because some option strings need extra quotation marks, which can't be easily done with your suggestion.

[andschar]

to vectorise the function I now use a for loop instead of lapply(). This might sound "un-Rish" at first, however I think in this case it's much better because a for loop doesn't return anything. Hence, if I run a = cir_img('CCO', tempdir()), a will be NULL. If I were to use lapply() I'd receive some sort of list, depending on whether all results return errors or not. Also, to avoid printing something to the console, I'd have to wrap the whole function into invisible().

[Aariq]

One more change and I think it'll probably be good to go. It currently has the issue where it mistakes NA for sodium.

>cir_img(NA, here())
Querying NA. OK (HTTP 200). Image saved under: /Users/scottericr/Documents/webchem/NA.png

[stitam]

Thanks @andschar for your work on this PR and sorry for the late reply. I agree, this PR is almost there.

[andschar]

One more change and I think it'll probably be good to go. It currently has the issue where it mistakes NA for sodium.

>cir_img(NA, here())
Querying NA. OK (HTTP 200). Image saved under: /Users/scottericr/Documents/webchem/NA.png

Good point. It's also a problem if somebody would call the function like this cir_img('', tempdir()), because an empty image would be returned. My solution would be the following. Check NAs and empty strings at the start of the function. What do you think? Are there better options?

vec = c(NA, '', 'Triclosan')
if (anyNA(vec) || any(vec == '')) {
  stop('NA or empty strings provided.')
}

[stitam]

Checks fail because Pubchem is a living database so tests often become outdated. Let's not worry about those tests here. CIR tests work locally.