search

ropensci / webchem

Chemical Information from the Web
https://docs.ropensci.org/webchem
Other
160 stars 40 forks source link

Fix pc_sect() failing on certain CIDs #392

Closed BrianFFish closed 1 year ago

BrianFFish commented 1 year ago

Brief description of the PR

Fix for #354. @stitam and @sxthimons confirmed that the cause of pc_sect() sometimes failing with certain CIDs was that certain PubChem records would return NA by httr::content(), and NA as the txt argument in jsonlite::fromJSON() results in an error that halts code execution. Since pc_sect() is a function likely to be run in a loop, it seems better if pc_sect() fails silently on NA returned from httr::content() and continues instead of breaking the loop with an opaque error.

PR task list:

stitam commented 1 year ago

Thanks @BrianFFish for opening this PR. I also started working on this issue yesterday but I needed to ensure the tests pass as well. Let's collaborate on this PR! I will mark it as draft, push my own edits, fix the rest of the tests, and then we can review and merge.

stitam commented 1 year ago

On second thought, I will fix the tests in a separate PR to keep things more tidy.

Aariq commented 1 year ago

Thanks for the fix. I just wanted to mention that pc_sect() is vectorized, so you don't need a for-loop to run it on multiple compounds—just if you want to get data from multiple "sections"