Skip to content
This repository was archived by the owner on Sep 9, 2022. It is now read-only.

Grabbing supplementary data from articles#62

Merged
sckott merged 5 commits into
ropensci-archive:masterfrom
willpearse:master
Sep 10, 2015
Merged

Grabbing supplementary data from articles#62
sckott merged 5 commits into
ropensci-archive:masterfrom
willpearse:master

Conversation

@willpearse
Copy link
Copy Markdown
Contributor

...this is the pull request for issue #61.

Sorry for the delay; 'tis the job season to be jolly, etc...

I've refactored etc. to make everything look a bit more like the rest of the package. Let me know what you think; I won't in any way be offended if you don't want it in. There isn't perfect overlap between what you can get SI for and what you can get text for; I plan to get around to fixing that (...) but I reasoned you would rather have something than nothing!

I've made some changes to the .Rmd files as well, but for some reason I was having a lot of trouble building them (some sort of pandocs problem getting access to a server). I'm a Linux person, so I'm certain this reflects something weird that I've done.

@sckott
Copy link
Copy Markdown
Contributor

sckott commented Sep 1, 2015

Awesome! Thanks.

On vacation now. Back next monday.

One thing I see so far. Can you use httr instead of rcurl? Don't want to introduce new dependencies.

@willpearse
Copy link
Copy Markdown
Contributor Author

Done. I also fixed a weird build problem. Apparently I'm rubbish at RMarkdown...!

Enjoy your holiday - stop checking GitHub :p

@sckott sckott self-assigned this Sep 9, 2015
@sckott
Copy link
Copy Markdown
Contributor

sckott commented Sep 9, 2015

looking at this now

Comment thread R/ft_get_si.R Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please wrap examples in \dontrun{}

@willpearse
Copy link
Copy Markdown
Contributor Author

Hopefully this is everything you asked for!

@sckott
Copy link
Copy Markdown
Contributor

sckott commented Sep 9, 2015

thanks 👍

@sckott
Copy link
Copy Markdown
Contributor

sckott commented Sep 9, 2015

@willpearse that one test is still failing, can you fix that

@willpearse
Copy link
Copy Markdown
Contributor Author

The problem stems from my output from rcrossref::cr_works being different over the last few days. I can just refactor to fix this, but I'm concerned that I'm just confused about how this function works. Why is it that when I run the following:

cr_works(dois="not-a-DOI")

...I get something back? When I first wrote this I'm sure 'nonsense' DOIs returned with no data whatsoever, and the wrong result I get has changed in the last 24 hours.I wrote it, initially, to give two different error messages - one for when I couldn't find the right publisher, and the other for when I found the publisher but couldn't download from them.

Has something shifted on the crossref API, or am I just being stupid (I sense it's the latter!)

@sckott
Copy link
Copy Markdown
Contributor

sckott commented Sep 10, 2015

@willpearse this is a bug in the Crossref API,

This is the API call from your e.g., http://api.crossref.org/works?query=not-a-DOI which gives that same result.

Here's a related issue CrossRef/rest-api-doc#37 in the repo for their API

Hopefully they'll get that fixed soon.

So I guess a change in your test to account for that would be good.

@willpearse
Copy link
Copy Markdown
Contributor Author

Done. Thanks for showing me the bug!

sckott added a commit that referenced this pull request Sep 10, 2015
Grabbing supplementary data from articles
@sckott sckott merged commit 0e1df2c into ropensci-archive:master Sep 10, 2015
@willpearse
Copy link
Copy Markdown
Contributor Author

👍 thanks!

@sckott
Copy link
Copy Markdown
Contributor

sckott commented Sep 10, 2015

thank you for this!

@sckott sckott added this to the v0.1.4 milestone Oct 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants