Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix get_cid to return NA when query is NA and style update #223

Merged
merged 3 commits into from Mar 17, 2020

Conversation

stitam
Copy link
Contributor

@stitam stitam commented Mar 17, 2020

Most of the PR is just style update according to lintr::lint(). The real difference is in line 59 of pubchem.R. get_cid() returned the Pubchem ID of sodium when the query was NA.

PR task list:

  • Update NEWS
  • Add tests (if appropriate)
  • Update documentation with devtools::document()
  • Check package passed

@codecov-io
Copy link

codecov-io commented Mar 17, 2020

Codecov Report

Merging #223 into master will not change coverage by %.
The diff coverage is 0.00%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #223   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          19      19           
  Lines        1582    1587    +5     
======================================
- Misses       1582    1587    +5     
Impacted Files Coverage Δ
R/chemspider.R 0.00% <ø> (ø)
R/pubchem.R 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67b0603...9aff121. Read the comment docs.

@Aariq
Copy link
Collaborator

Aariq commented Mar 17, 2020

Nice! We should probably be careful about proper handling of NA for all get_ functions, not just because of the sodium issue (which has shown up before), but because we want them to be properly vectorized.

@Aariq Aariq merged commit d02b3bc into ropensci:master Mar 17, 2020
@stitam stitam added this to the RC2019F milestone Sep 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants