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

added CAS, EC, GSBL, RTECS search options to get_etoxid() #241

Merged
merged 6 commits into from Apr 24, 2020

Conversation

andschar
Copy link
Contributor

@andschar andschar commented Apr 20, 2020

I've added features to query the ETOX data-base by CAS, EC, GSBL, RTECS numbers in addition to the already existing option to search for names (as discussed in #237).

NB: I've added examples and tests for all the new features except for RETCS-numbers, which seems to be a proprietary format, for which I only found rather old publications. I left it there nonetheless.

PR task list:

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

@codecov-io
Copy link

codecov-io commented Apr 20, 2020

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #241   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          19      19           
  Lines        1672    1685   +13     
======================================
- Misses       1672    1685   +13     
Impacted Files Coverage Δ
R/etox.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 9e32bd8...7b86691. Read the comment docs.

NEWS Outdated Show resolved Hide resolved
R/etox.R Show resolved Hide resolved
R/etox.R Show resolved Hide resolved
Copy link
Collaborator

@Aariq Aariq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested it out, but looks great in format. I think there needs to be a bit more documentation for the lesser-known options for from=.

R/etox.R Show resolved Hide resolved
R/etox.R Outdated Show resolved Hide resolved
@andschar
Copy link
Contributor Author

andschar commented Apr 20, 2020

Sorry for the mess. Don't know why travis check is failing. Could it be because of man/nist_ri.Rd? It doesn't seem to be built.

@stitam
Copy link
Contributor

stitam commented Apr 21, 2020

According to log it has to do with cts_compinfo(), the server seems to be down. I think @Aariq had the same problem in PR #232. While this is something we have to know about, maybe it shouldn't cause a build to fail. I suggest we ignore if Travis is failing because of cts_compinfo() but open a separate issue for the topic and discuss how to handle these situations.

@Aariq
Copy link
Collaborator

Aariq commented Apr 22, 2020

According to log it has to do with cts_compinfo(), the server seems to be down. I think @Aariq had the same problem in PR #232. While this is something we have to know about, maybe it shouldn't cause a build to fail. I suggest we ignore if Travis is failing because of cts_compinfo() but open a separate issue for the topic and discuss how to handle these situations.

It only fails on the development build of R. It passes with the release build of R. I can't figure out why either.

R/etox.R Show resolved Hide resolved
@stitam stitam merged commit 36c9674 into ropensci:master Apr 24, 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

4 participants